Conversation
Youssef1313
force-pushed
the
dev/ygerges/mtp
branch
from
8e80eee to
045095b
Compare
Youssef1313
force-pushed
the
dev/ygerges/mtp
branch
from
fe653b5 to
65d17a8
Compare
|
fyi I'm working on this in #13215, but won't be mad if you beat me to it. Chasing what looks like some Windows hangs at the moment, and as a low-pri background thing. |
|
Oh, I didn't know you are already working on it. |
|
@rainersigwald I tried to pull in some changes you did in the other PR here to see if it gets me here in a better state. We can sync next Tuesday to align the efforts. Generally speaking, a big difference between the two PRs is that in this PR I'm running with MTP, while the other PR runs with VSTest. But I would think most test failures will be related to the xunit 2 to xunit 3 move and not VSTest/MTP differences. In addition, in this PR I'm keeping the tests wrapped in One thing to note is that xunit.v3 might be running the tests in a different order compared to xunit 2, which might (or might not) be the cause for some of the failures (i.e, some tests might have been modifying global state without resetting it - that was somehow unnoticed with xunit 2 but might become apparent now that the ordering changed) |
I believe this is not currently working--my interpretation of a bunch of failures in my attempt was "something is making this work now and failing a bunch of tests that should have been failing before but weren't". I'd love to have all those tests fixed of course . . . |
In the other PR, I commented on why the approach there won't work. But I think it might have been working on main already. |
…auses some caches to be bad/unexpected for tests.
Youssef1313
force-pushed
the
dev/ygerges/mtp
branch
from
ec93ec7 to
6f93f7e
Compare
| var exceptionFilesBefore = | ||
| Directory.Exists(DebugUtils.DebugDumpPath) ? Directory.GetFiles(DebugUtils.DebugDumpPath, "MSBuild_*failure.txt") : Array.Empty<string>(); |
There was a problem hiding this comment.
For some unknown reason, sometimes the DebugDumpPath directory isn't there on disk, and then GetFiles throws an exception if the directory doesn't exist.
Maybe there is some other previous test that deletes DebugDumpPath or something. Anyways, it should be safe to keep this change. It just means that future refactoring for how tests work is ideal to avoid this kind of cross-test dependency issues.
| private static string AssemblyLocation { get; } = | ||
| typeof(TaskHostFactory_Tests).Assembly.Location | ||
| ?? Path.Combine(AppContext.BaseDirectory, "Microsoft.Build.Engine.UnitTests.dll"); |
There was a problem hiding this comment.
- NOTE 1: Under .NET Framework, the assembly is now the
.exewhichAssembly.Locationwill provide correctly. - NOTE 2: In the case of
Assembly.Locationbeing null (unlikely to happen for .NET Framework), we still useAppContext.BaseDirectoryand then get the dll. If this ever happened for .NET Framework, we would need to add extra condition to either find.dllor.exefor .NET Framework. But so farAssembly.Locationsimply works just fine.
| ParallelConsoleLogger cl2 = new ParallelConsoleLogger(LoggerVerbosity.Minimal, sc.Write, null, null); | ||
| EventSourceSink es = new EventSourceSink(); | ||
| cl2.Initialize(es); | ||
| cl2.Parameters = "ShowEnvironment"; |
There was a problem hiding this comment.
NOTE: This line will lead to:
So, we ensure we get back the static state to its original value (should be false).
There is some other test that would fail if Traits.LogAllEnvironmentVariables is true (I don't recall which one now).
In current main, DisplayEnvironmentInMinimal is run after that test so everything kinda works fine.
With xunit.v3, the order changed and now DisplayEnvironmentInMinimal is run earlier, causing the other test to fail.
2 participants
No description provided.