Skip to content

Conversation

@stevenebutler
Copy link
Contributor

@stevenebutler stevenebutler commented Apr 18, 2023

Closes #19531

  • Lengthen the race window from milliseconds to seconds.
  • Don't sleep any longer than necessary if a task has already completed.

PR Summary

Addresses request by @daxian-dbw in #19330

Raised as #19531

PR Context

CTRL C tests rely races on timeouts that had time-frames in 100s of milliseconds. These may be too tight when running in CI where resources are stretched. Increased the timeouts and changed logic so it only waits as long as necessary if possible.

PR Checklist

Lengthen the race window from milliseconds to seconds.
Don't sleep any longer than necessary if a task has already completed.
@ghost ghost assigned anmenaga Apr 18, 2023
@stevenebutler
Copy link
Contributor Author

I'm seeing failure of an unrelated test on MacOS:
Test-Connection.Ping.Quiet works
at <ScriptBlock>, /Users/runner/work/1/s/test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1: line 90
90: $result2 | Should -BeFalse

@stevenebutler stevenebutler changed the title Improve reliability of CTRL-C tests Invoke-WebRequest/RestMethod: Improve reliability of CTRL-C tests Apr 19, 2023
@stevenebutler
Copy link
Contributor Author

@daxian-dbw I have made all changes as suggested in case this is exactly what you wanted. Let me know if you want me to dial back the delay a bit so the tests don't take so long to run. I split the brotli/deflate/gzip tests into separate cases as well.

Copy link
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification and reverting the changes.

Copy link
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @stevenebutler!

@daxian-dbw daxian-dbw merged commit 319c9fa into PowerShell:master Apr 20, 2023
@daxian-dbw daxian-dbw added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Apr 20, 2023
@ghost
Copy link

ghost commented Jun 29, 2023

🎉v7.4.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request Jun 29, 2023
@stevenebutler stevenebutler deleted the improve-iwr-ctrlc-test-reliability branch January 4, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Test Indicates that a PR should be marked as a test change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve reliability of CTRL-C tests

3 participants