Skip to content

Commit

Permalink
Remove patching NUnit internal logger (#1171)
Browse files Browse the repository at this point in the history
We couldn't reproduce the issue we identified when we started to work
on adding support for NUnit on our new platform.
After some tests with framework v3 and v4 in command line and in VS,
I wasn't able to reproduce any issue so I am removing the patching code.

Note that the field was renamed in v4 (starts with some underscore)
which is causing the code to fail on line 106
  • Loading branch information
Evangelink authored May 6, 2024
1 parent eb208f0 commit 4010a76
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 60 deletions.
2 changes: 1 addition & 1 deletion build.cake
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var configuration = Argument("configuration", "Release");

var version = "5.0.0";

var modifier = "-beta.1";
var modifier = "-beta.2";

var dbgSuffix = configuration.ToLower() == "debug" ? "-dbg" : "";
var packageVersion = version + modifier + dbgSuffix;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -12,20 +10,14 @@
using Microsoft.Testing.Platform.Messages;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Adapter;

using NUnit.Framework.Internal;

namespace NUnit.VisualStudio.TestAdapter.TestingPlatformAdapter
{
internal sealed class NUnitBridgedTestFramework : SynchronizedSingleSessionVSTestBridgedTestFramework
{
private static readonly object InitializationLock = new();
private static bool initialized;

public NUnitBridgedTestFramework(NUnitExtension extension, Func<IEnumerable<Assembly>> getTestAssemblies,
IServiceProvider serviceProvider, ITestFrameworkCapabilities capabilities)
: base(extension, getTestAssemblies, serviceProvider, capabilities)
{
PatchNUnit3InternalLoggerBug();
}

/// <inheritdoc />
Expand Down Expand Up @@ -57,56 +49,5 @@ protected override Task SynchronizedRunTestsAsync(VSTestRunTestExecutionRequest

return Task.CompletedTask;
}

private static void PatchNUnit3InternalLoggerBug()
{
// NUnit3 will internally call InternalTracer.Initialize(...), and it will check that it is not called multiple times.
// We call it multiple times in server mode, in indeterminate order (it does not always have to be Discover and then Run),
// When InternalTracer.Initialize is called with Off, it will not set traceWriter internal field to anything, but when you call it again
// it will try to write "Initialize was already called" via this writer without null check and will fail with NRE.
//
// Patch for this was implemented in NUnit4, but won't be backported to NUnit3 because we are hitting a bit of an edge case:
// https://github.com/nunit/nunit/issues/4255
//
// To fix us, we set the writer to a null writer, so any subsequent calls to Initialize will write to null instead of failing.
// We also need to do this under a lock, and not rely on the InternalTracer.Initialized, because that might be set to late and we would
// re-enter the method twice.
if (initialized)
{
return;
}

lock (InitializationLock)
{
if (initialized)
{
return;
}

var nopWriter = new InternalTraceWriter(new StreamWriter(Stream.Null));

// Initialize log level to be Off (see issue https://github.com/microsoft/testanywhere/issues/1369)
// because we don't have settings from the request available yet, and the internal tracer is static, so it would set to the
// level that is set by the first request and always keep it that way.
//
// Alternatively we could hook this up to a ILogger, and write the logs via a TextWriter.
InternalTrace.Initialize(nopWriter, InternalTraceLevel.Off);

// When we allow the trace level to be set then we need to set the internal writer field only when the level is Off.
// In that case you will need to do something like this:
// FieldInfo traceLevelField = typeof(InternalTrace).GetField("traceLevel", BindingFlags.Static | BindingFlags.NonPublic)!;
// bool isOff = ((InternalTraceLevel)traceLevelField.GetValue(null)!) != InternalTraceLevel.Off;
// if (isOff)
// {
// FieldInfo traceWriterField = typeof(InternalTrace).GetField("traceWriter", BindingFlags.Static | BindingFlags.NonPublic)!;
// traceWriterField.SetValue(null, nopWriter);
// }

FieldInfo traceWriterField = typeof(InternalTrace).GetField("traceWriter", BindingFlags.Static | BindingFlags.NonPublic)!;
traceWriterField.SetValue(null, nopWriter);

initialized = true;
}
}
}
}

0 comments on commit 4010a76

Please sign in to comment.