From 4010a76b64679abaf721a5fc2bf242e5091f584a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Amaury=20Lev=C3=A9?= <amauryleve@microsoft.com>
Date: Mon, 6 May 2024 22:14:55 +0200
Subject: [PATCH] Remove patching NUnit internal logger (#1171)

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
---
 build.cake                                    |  2 +-
 .../NUnitBridgedTestFramework.cs              | 59 -------------------
 2 files changed, 1 insertion(+), 60 deletions(-)

diff --git a/build.cake b/build.cake
index f5f4fd15..874a0979 100644
--- a/build.cake
+++ b/build.cake
@@ -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;
diff --git a/src/NUnitTestAdapter/TestingPlatformAdapter/NUnitBridgedTestFramework.cs b/src/NUnitTestAdapter/TestingPlatformAdapter/NUnitBridgedTestFramework.cs
index 23c8b159..f53a2904 100644
--- a/src/NUnitTestAdapter/TestingPlatformAdapter/NUnitBridgedTestFramework.cs
+++ b/src/NUnitTestAdapter/TestingPlatformAdapter/NUnitBridgedTestFramework.cs
@@ -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;
@@ -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 />
@@ -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;
-            }
-        }
     }
 }