Skip to content

Commit 6f93f7e

Browse files
Youssef1313Youssef1313
committed
Additional fixes due to test order changes
1 parent c0e2230 commit 6f93f7e

File tree

2 files changed

+86
-56
lines changed

2 files changed

+86
-56
lines changed

‎src/Build.UnitTests/BinaryLogger_Tests.cs‎

Lines changed: 80 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -101,76 +101,100 @@ public enum BinlogRoundtripTestReplayMode
101101
[InlineData(s_testProject2, BinlogRoundtripTestReplayMode.RawEvents)]
102102
public void TestBinaryLoggerRoundtrip(string projectText, BinlogRoundtripTestReplayMode replayMode)
103103
{
104-
var binaryLogger = new BinaryLogger();
104+
// NOTE:
105+
// We want both loggers to see the same value of EnableTargetOutputLogging, otherwise, the last assertion will fail.
106+
// See logic around showTargetOutputs.
107+
// In short, this controls whether or not the "Target output items:" part is shown.
108+
// Traits.Instance is weird, it's not always a singleton, depending on whether or not BuildEnvironmentState.s_runningTests is true.
109+
// The s_runningTests check is also not that straightforward.
110+
// Calls to ResetInstance_ForUnitTestsOnly (by tests that ran earlier than this test) might end up setting s_runningTests to false.
111+
// So the current approach is extremely vulnerable to the order of tests being run, and also for test filters.
112+
// When s_runningTests is true, we don't use a singleton, and in this case, one logger shows target outputs while the other doesn't.
113+
// When s_runningTests is false, Traits.Instance is truly a singleton and so both see the same value.
114+
// To stabilize this specific test, we explicitly set EnableTargetOutputLogging to true so we guarantee that both loggers see the same value.
115+
// Long-term, unifying s_runningTests to always be true in unit tests (or even removing it completely, if possible) is the best approach.
116+
var initialEnableTargetOutputLogging = Traits.Instance.EnableTargetOutputLogging;
117+
using var env = TestEnvironment.Create();
118+
try
119+
{
120+
env.SetEnvironmentVariable("MSBUILDTARGETOUTPUTLOGGING", "1");
121+
Traits.Instance.EnableTargetOutputLogging = true;
105122

106-
binaryLogger.Parameters = _logFile;
123+
var binaryLogger = new BinaryLogger();
107124

108-
var mockLogFromBuild = new MockLogger();
125+
binaryLogger.Parameters = _logFile;
109126

110-
var parallelFromBuildText = new StringBuilder();
111-
var parallelFromBuild = new ParallelConsoleLogger(Framework.LoggerVerbosity.Diagnostic, t => parallelFromBuildText.Append(t), colorSet: null, colorReset: null);
112-
parallelFromBuild.Parameters = "NOPERFORMANCESUMMARY";
127+
var mockLogFromBuild = new MockLogger();
113128

114-
// build and log into binary logger, mock logger and parallel console loggers
115-
// no logging on evaluation
116-
using (ProjectCollection collection = new())
117-
{
118-
Project project = ObjectModelHelpers.CreateInMemoryProject(collection, projectText);
119-
project.Build(new ILogger[] { binaryLogger, mockLogFromBuild, parallelFromBuild }).ShouldBeTrue();
120-
}
129+
var parallelFromBuildText = new StringBuilder();
130+
var parallelFromBuild = new ParallelConsoleLogger(Framework.LoggerVerbosity.Diagnostic, t => parallelFromBuildText.Append(t), colorSet: null, colorReset: null);
131+
parallelFromBuild.Parameters = "NOPERFORMANCESUMMARY";
121132

122-
string fileToReplay;
123-
switch (replayMode)
124-
{
125-
case BinlogRoundtripTestReplayMode.NoReplay:
126-
fileToReplay = _logFile;
127-
break;
128-
case BinlogRoundtripTestReplayMode.Structured:
129-
case BinlogRoundtripTestReplayMode.RawEvents:
130-
{
131-
var logReader = new BinaryLogReplayEventSource();
132-
fileToReplay = _env.ExpectFile(".binlog").Path;
133-
if (replayMode == BinlogRoundtripTestReplayMode.Structured)
134-
{
135-
// need dummy handler to force structured replay
136-
logReader.BuildFinished += (_, _) => { };
137-
}
133+
// build and log into binary logger, mock logger and parallel console loggers
134+
// no logging on evaluation
135+
using (ProjectCollection collection = new())
136+
{
137+
Project project = ObjectModelHelpers.CreateInMemoryProject(collection, projectText);
138+
project.Build(new ILogger[] { binaryLogger, mockLogFromBuild, parallelFromBuild }).ShouldBeTrue();
139+
}
138140

139-
BinaryLogger outputBinlog = new BinaryLogger()
141+
string fileToReplay;
142+
switch (replayMode)
143+
{
144+
case BinlogRoundtripTestReplayMode.NoReplay:
145+
fileToReplay = _logFile;
146+
break;
147+
case BinlogRoundtripTestReplayMode.Structured:
148+
case BinlogRoundtripTestReplayMode.RawEvents:
140149
{
141-
Parameters = fileToReplay
142-
};
143-
outputBinlog.Initialize(logReader);
144-
logReader.Replay(_logFile);
145-
outputBinlog.Shutdown();
146-
}
147-
break;
148-
default:
149-
throw new ArgumentOutOfRangeException(nameof(replayMode), replayMode, null);
150-
}
150+
var logReader = new BinaryLogReplayEventSource();
151+
fileToReplay = _env.ExpectFile(".binlog").Path;
152+
if (replayMode == BinlogRoundtripTestReplayMode.Structured)
153+
{
154+
// need dummy handler to force structured replay
155+
logReader.BuildFinished += (_, _) => { };
156+
}
157+
158+
BinaryLogger outputBinlog = new BinaryLogger()
159+
{
160+
Parameters = fileToReplay
161+
};
162+
outputBinlog.Initialize(logReader);
163+
logReader.Replay(_logFile);
164+
outputBinlog.Shutdown();
165+
}
166+
break;
167+
default:
168+
throw new ArgumentOutOfRangeException(nameof(replayMode), replayMode, null);
169+
}
151170

152-
var mockLogFromPlayback = new MockLogger();
171+
var mockLogFromPlayback = new MockLogger();
153172

154-
var parallelFromPlaybackText = new StringBuilder();
155-
var parallelFromPlayback = new ParallelConsoleLogger(Framework.LoggerVerbosity.Diagnostic, t => parallelFromPlaybackText.Append(t), colorSet: null, colorReset: null);
156-
parallelFromPlayback.Parameters = "NOPERFORMANCESUMMARY";
173+
var parallelFromPlaybackText = new StringBuilder();
174+
var parallelFromPlayback = new ParallelConsoleLogger(Framework.LoggerVerbosity.Diagnostic, t => parallelFromPlaybackText.Append(t), colorSet: null, colorReset: null);
175+
parallelFromPlayback.Parameters = "NOPERFORMANCESUMMARY";
157176

158-
var binaryLogReader = new BinaryLogReplayEventSource();
159-
mockLogFromPlayback.Initialize(binaryLogReader);
160-
parallelFromPlayback.Initialize(binaryLogReader);
177+
var binaryLogReader = new BinaryLogReplayEventSource();
178+
mockLogFromPlayback.Initialize(binaryLogReader);
179+
parallelFromPlayback.Initialize(binaryLogReader);
161180

162-
// read the binary log and replay into mockLogger2
163-
binaryLogReader.Replay(fileToReplay);
164-
mockLogFromPlayback.Shutdown();
165-
parallelFromPlayback.Shutdown();
181+
// read the binary log and replay into mockLogger2
182+
binaryLogReader.Replay(fileToReplay);
183+
mockLogFromPlayback.Shutdown();
184+
parallelFromPlayback.Shutdown();
166185

167-
// the binlog will have more information than recorded by the text log
168-
mockLogFromPlayback.FullLog.ShouldContainWithoutWhitespace(mockLogFromBuild.FullLog);
186+
// the binlog will have more information than recorded by the text log
187+
mockLogFromPlayback.FullLog.ShouldContainWithoutWhitespace(mockLogFromBuild.FullLog);
169188

170-
var parallelExpected = parallelFromBuildText.ToString();
171-
var parallelActual = parallelFromPlaybackText.ToString();
189+
var parallelExpected = parallelFromBuildText.ToString();
190+
var parallelActual = parallelFromPlaybackText.ToString();
172191

173-
parallelActual.ShouldContainWithoutWhitespace(parallelExpected);
192+
parallelActual.ShouldContainWithoutWhitespace(parallelExpected);
193+
}
194+
finally
195+
{
196+
Traits.Instance.EnableTargetOutputLogging = initialEnableTargetOutputLogging;
197+
}
174198
}
175199

176200
/// <summary>

‎src/Build.UnitTests/Construction/ProjectRootElement_Tests.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,12 @@ public void ProjectLoadedStrippingCommentsAndWhiteSpaceIsReadOnly()
115115
Assert.Equal(2, children[0].ChildNodes.Count);
116116
Assert.Equal(string.Empty, children[0].ChildNodes[0].Value);
117117
Assert.Equal(string.Empty, children[0].ChildNodes[1].Value);
118+
119+
// We cleared at the beginning, but then we set MSBUILDLOADALLFILESASREADONLY to 1.
120+
// This means that opening the project will cache s_readOnlyFlags as ReadOnlyLoadFlags.LoadAllReadOnly
121+
// Keeping the cached ReadOnlyLoadFlags.LoadAllReadOnly can impact subsequent tests that are running in the same process.
122+
// So ensure to re-clear.
123+
XmlDocumentWithLocation.ClearReadOnlyFlags_UnitTestsOnly();
118124
}
119125
}
120126

0 commit comments

Comments
 (0)