Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove patching NUnit internal logger #1171

Merged
merged 1 commit into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
}
}
}