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

[release/6.0.1xx] Re-write dotnet-watch smoke test #14367

Merged

Conversation

lbussell
Copy link

Our dotnet watch smoke tests are flaky. We keep getting test failures even when dotnet watch is functioning as expected. It seems that the exit code when sending a kill command is not a good indicator of whether or not dotnet watch was functioning properly.

Currently we check that the correct text is output after editing a file that is being watched, and then kill the program and check the exit code. I'm attempting to change the test so that we succeed if and only if the expected text is output by the program, and disregarding the exit code. We should fail the test if we never see the expected output and then timeout.

At the moment I'm not sure if we will fail a test when we timeout. I'm submitting early as a draft to get feedback.

@lbussell lbussell requested a review from MichaelSimons August 22, 2022 17:01
@MichaelSimons
Copy link
Member

At the moment I'm not sure if we will fail a test when we timeout.

Couldn't you just change the expectedString to something that will never happen and then run the test locally?

OutputHelper.WriteLine("Successfully re-ran program after code change.");
ExecuteHelper.ExecuteProcessValidateExitCode("kill", $"-s TERM {process.Id}", OutputHelper);
process.Kill(true);
tcs.TrySetResult(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the value in using a TaskCompletionSource here versus just setting a boolean at this point similar to the original code?

@lbussell
Copy link
Author

I merged in 6.0.1xx and also tested that this can fail using Michael's suggestion above, so this is ready for review.

@lbussell lbussell marked this pull request as ready for review October 11, 2022 23:30
@lbussell lbussell requested a review from a team as a code owner October 11, 2022 23:30
@MichaelSimons MichaelSimons enabled auto-merge (squash) October 12, 2022 00:46
@MichaelSimons MichaelSimons merged commit 7aec869 into dotnet:release/6.0.1xx Oct 12, 2022
lbussell added a commit to lbussell/installer that referenced this pull request Oct 12, 2022
* Re-write dotnet-watch smoke test

* Revert async changes
MichaelSimons added a commit that referenced this pull request Oct 12, 2022
* [release/6.0.1xx] Enable online and offline source-build tarball build in all PRs (#14711)

* Enable online and offline source-build tarball build in all PRs (#14620)

* Fix source-build CI to not run tarball build when tarball creation fails (#14678)

* Fix source-build CI to not run tarball build when tarball creation failed

* Bad fix

* Revert test patch

* update global.json and Versions.props for .NET SDK 6.0.110 (#14717)

* [release/6.0.1xx] Re-write dotnet-watch smoke test (#14367)

* Re-write dotnet-watch smoke test

* Revert async changes

* Disable source-build tarball validation for non 6.0.1xx legs

Co-authored-by: Michael Simons <msimons@microsoft.com>
Co-authored-by: NET Source-Build Bot <102560831+dotnet-sb-bot@users.noreply.github.com>
Co-authored-by: Logan Bussell <loganbussell@microsoft.com>
Co-authored-by: Jason Zhai <v-wuzhai@microsoft.com>
MichaelSimons pushed a commit that referenced this pull request Oct 13, 2022
* [release/6.0.1xx] Re-write dotnet-watch smoke test (#14367)

* Re-write dotnet-watch smoke test

* Revert async changes

* Re-enable dotnet-watch test
MichaelSimons added a commit that referenced this pull request Oct 13, 2022
* [automated] Merge branch 'release/6.0.1xx' => 'release/6.0.3xx' (#14726)

* [release/6.0.1xx] Enable online and offline source-build tarball build in all PRs (#14711)

* Enable online and offline source-build tarball build in all PRs (#14620)

* Fix source-build CI to not run tarball build when tarball creation fails (#14678)

* Fix source-build CI to not run tarball build when tarball creation failed

* Bad fix

* Revert test patch

* update global.json and Versions.props for .NET SDK 6.0.110 (#14717)

* [release/6.0.1xx] Re-write dotnet-watch smoke test (#14367)

* Re-write dotnet-watch smoke test

* Revert async changes

* Disable source-build tarball validation for non 6.0.1xx legs

Co-authored-by: Michael Simons <msimons@microsoft.com>
Co-authored-by: NET Source-Build Bot <102560831+dotnet-sb-bot@users.noreply.github.com>
Co-authored-by: Logan Bussell <loganbussell@microsoft.com>
Co-authored-by: Jason Zhai <v-wuzhai@microsoft.com>

* Update asp.net templates (#14687)

* Update asp.net templates (#14688)

Co-authored-by: Michael Simons <msimons@microsoft.com>
Co-authored-by: NET Source-Build Bot <102560831+dotnet-sb-bot@users.noreply.github.com>
Co-authored-by: Logan Bussell <loganbussell@microsoft.com>
Co-authored-by: Jason Zhai <v-wuzhai@microsoft.com>
Co-authored-by: Marc Paine <marcpop@microsoft.com>
Co-authored-by: William Godbe <wigodbe@microsoft.com>
Co-authored-by: v-wuzhai <46013274+v-wuzhai@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants