[net11.0] 'dotnet watch' works again now, so enable the corresponding tests.#25712
[net11.0] 'dotnet watch' works again now, so enable the corresponding tests.#25712rolfbjarne wants to merge 3 commits into
Conversation
… tests. Also a few improvements, because this test turned out rather unpredictable locally: * Detect if the build failed, and terminate the test early. The build shouldn't fail, but yet it randomly does, and this makes the test fail immediately instead of having the test time out (after a 2 min wait). * Don't give stdin to 'dotnet watch', because 'dotnet test' goes bananas. Deadlocks everywhere (dozens of threads stuck while trying to determine the cursor position in the terminal, etc). * Disable build servers. Not sure if it helps, but when random things fail, random stuff gets tried, and this won't hurt. * Don't run desktop test apps with 'open', because then the test app's process is not a descendent of the 'dotnet watch' process, and if something goes wrong and the test needs to kill 'dotnet watch', the test app won't be killed. * Always cancel the 'dotnet watch' process at the end. Copilot suggested doing this, and it sounded good so here it is. * Augment the 'Execution' class to kill the entire process tree. Once again, Copilot suggested doing this, and it sounds like a good thing to do. * Add CoreCLR mobile variations.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR re-enables and expands the dotnet watch unit tests on the net11.0 branch, while improving test reliability and process handling in the shared Execution helper used by tests/tools.
Changes:
- Extend
Executionto optionally close stdin for spawned processes, and to kill process trees on cancellation/timeout (where supported). - Re-enable/expand
DotNetWatchTestacross additional Apple platforms and add CoreCLR/Mono runtime variations. - Improve
DotNetWatchTestrobustness (detect initial build failures earlier, avoid stdin deadlocks, and adjust cleanup behavior).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tools/common/Execution.cs | Adds stdin-closing option and introduces process-tree kill helper used on cancellation/timeout. |
| tests/dotnet/UnitTests/DotNetWatchTest.cs | Re-enables/enlarges dotnet watch coverage and hardens the test flow & cleanup. |
| Log?.WriteLine ($"Command '{p.StartInfo.FileName} {p.StartInfo.Arguments}' (pid: {pid}) was cancelled, and will be killed."); | ||
| KillProcess (p, pid, Log); | ||
| } catch (Exception ex) { |
| Log?.WriteLine ($"Command '{p.StartInfo.FileName} {p.StartInfo.Arguments}' (pid: {pid}) didn't finish in {Timeout.Value.TotalMilliseconds} ms, and will be killed."); | ||
| TimedOut = true; | ||
| try { | ||
| p.Kill (); | ||
| KillProcess (p, pid, Log); | ||
| } catch (Exception ex) { |
| // Wait for the app to start and show initial output | ||
| debugLog.WriteLine ("Waiting for app start..."); | ||
| if (!appStarted.Task.Wait (TimeSpan.FromMinutes (1))) | ||
| Assert.Fail ($"Timed out waiting for the app to start. Output:\n{string.Join ("\n", output)}\nDebug output:\n{string.Join ("\n", File.ReadAllLines (debugLogPath))}"); | ||
| debugLog.WriteLine ("App started!"); |
| try { | ||
| debugLog.WriteLine ("Waiting for exit..."); | ||
| watchTask.Wait (TimeSpan.FromSeconds (30)); | ||
| debugLog.WriteLine ("Waited for exit"); | ||
| } catch { | ||
| // Expected - the process was cancelled | ||
| } |
| debugLog.WriteLine ("Waiting for 'dotnet watch' to be waiting for changes..."); | ||
| if (!waitingForChanges.Task.Wait (TimeSpan.FromMinutes (1))) | ||
| Assert.Fail ($"Timed out waiting for the 'dotnet watch' to be waiting for changes. Output:\n{string.Join ("\n", output)}\nDebug output:\n{string.Join ("\n", File.ReadAllLines (debugLogPath))}"); | ||
| debugLog.WriteLine ("Waiting for changes!"); | ||
| Assert.That (waitingForChanges.Task.Result, Is.True, "Build failed"); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Remove unused pid/log parameters from KillProcess. - Detect build failures early while waiting for app start, instead of timing out after 1 minute. - Move 'Waiting for changes!' log after confirming the build succeeded. - Log exceptions and timeout in cleanup path instead of swallowing them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #e373f1e] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #e373f1e] Build passed (Build packages) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [PR Build #e373f1e] Build passed (Build macOS tests) ✅Pipeline on Agent |
🔥 [CI Build #e373f1e] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 36 tests failed, 169 tests passed. Failures❌ fsharp tests2 tests failed, 2 tests passed.Failed tests
Html Report (VSDrops) Download ❌ introspection tests6 tests failed, 3 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (tvOS)26 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Sonoma (14) tests2 tests failed, 3 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Sequoia (15): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Also a few improvements, because this test turned out rather unpredictable locally: