-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add retries to some FSW tests #68368
Conversation
danmoseley
commented
Apr 22, 2022
•
edited
Loading
edited
- Fix Rolling build test failure: System.IO.Tests.FileSystemWatcherTests.FileSystemWatcher_Directory_Create_MultipleFilters with message "Created event did not occur as expected" #67601 by adding retries to the ones that failed in the last 60 days. If we see this helps, we can add to others that fail or all of them.
- Fix Test failure: System.IO.Tests.FileSystemWatcherTests_netstandard17/EndInit_ResumesPausedEnableRaisingEvents(setBeforeBeginInit: True) #24181 and enable test
- Enable debugging output from RetryHelper showing the test that is being retried and why when an env var DEBUG_RETRYHELPER=1
- Enable console output from RetryHelper when in Helix and it's a FSW test. This will allow us to look at recent console logs to see whether retrying is happening and where.
- Remove previous custom retry method. The one test it was applied to has not recently failed.
Tagging subscribers to this area: @dotnet/area-system-io Issue Details
|
Since not one of these has failed on Mac although I see them running there (I guess we have a more robust approach on Mac) I could potentially disable the retry mechanism when on Mac. This is fine for now. |
src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Changed.cs
Show resolved
Hide resolved
src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Changed.cs
Show resolved
Hide resolved
src/libraries/System.Runtime.InteropServices.RuntimeInformation/tests/DescriptionNameTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new retries look good, but I do think the RetryHelper.Execute method that logs the diagnostic should be exclusive to FileSystemWatcher, to avoid an unnecessary Contains check on all tests that are have the helix bool returning true.
@@ -22,6 +23,8 @@ public static partial class PlatformDetection | |||
// do it in a way that failures don't cascade. | |||
// | |||
|
|||
public static readonly bool IsInHelix = Environment.GetEnvironmentVariables().Keys.Cast<string>().Where(key => key.StartsWith("HELIX")).Any(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be done lazily, similar to how other expensive properties in PlatformDetection are initialized lazily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll fix