Skip to content

Commit

Permalink
[Azure Search] Remove JsonConvert.DefaultSettings check from unit tes…
Browse files Browse the repository at this point in the history
…ts (#4053)

Some time ago, we added a check to every Azure Search unit test that uses the
Run() method. This check is intended to ensure that none of our SDK code uses
the global JsonConvert.DefaultSettings, since these can be altered by other
code in the same AppDomain. This was causing issues for customers, and is the
reason we added SafeJsonConvert to ClientRuntime and AutoRest.

The check in the Run() method installs a custom JsonConverter and
ContractResolver that throw exceptions whenever they're called. It does this
using a try/finally block so that the change should only be visible while
running the unit test.

Unfortunately, since JsonConvert.DefaultSettings is global state, this
approach is fragile. For example, running tests in parallel can break this
because some infrastructure such as the VS test runner framework itself uses
JSON.NET in ways that rely on DefaultSettings.

Given the fragility of this approach, we've decided to remove these checks
altogether since they were causing spurious test failures.
  • Loading branch information
brjohnstmsft authored and shahabhijeet committed Feb 7, 2018
1 parent 3d5f3be commit 194dfa9
Showing 1 changed file with 0 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ namespace Microsoft.Azure.Search.Tests.Utilities
using System.Runtime.CompilerServices;
using Microsoft.Azure.Management.Search;
using Microsoft.Rest.ClientRuntime.Azure.TestFramework;
using Newtonsoft.Json;
using Newtonsoft.Json.Serialization;

public abstract class SearchTestBase<TTestFixture> : TestBase where TTestFixture : IResourceFixture, new()
{
Expand Down Expand Up @@ -40,52 +38,15 @@ protected void Run(
Data = new TTestFixture();
Data.Initialize(mockContext);

Func<JsonSerializerSettings> oldDefault = JsonConvert.DefaultSettings;

// This should ensure that the SDK doesn't depend on global JSON.NET settings.
JsonConvert.DefaultSettings = () =>
new JsonSerializerSettings()
{
Converters = new[] { new InvalidJsonConverter() },
ContractResolver = new InvalidContractResolver()
};

try
{
testBody();
}
finally
{
Data.Cleanup();
JsonConvert.DefaultSettings = oldDefault;
}
}
}

private class InvalidContractResolver : IContractResolver
{
public JsonContract ResolveContract(Type type)
{
throw new InvalidOperationException(JsonErrorMessage);
}
}

private class InvalidJsonConverter : JsonConverter
{
public override bool CanConvert(Type objectType)
{
throw new InvalidOperationException(JsonErrorMessage);
}

public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
{
throw new InvalidOperationException(JsonErrorMessage);
}

public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
{
throw new InvalidOperationException(JsonErrorMessage);
}
}
}
}

0 comments on commit 194dfa9

Please sign in to comment.