- Notifications
You must be signed in to change notification settings - Fork1k
chore(agent/agentcontainers): skip part of test if ondarwin#19081
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Relates tocoder/internal#765I've been unable to figure out how to fix this (unfortunately my macOSfile system knowledge is lacking). Given that this test is causing a lotof flakes at the moment, I think it is best to skip this part of thetest on macOS for now.
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.
Pull Request Overview
This PR addresses test flakiness on macOS by conditionally skipping the atomic rename portion of a file system watcher test when running on Darwin (macOS). The change is intended as a temporary workaround for unreliable test behavior on macOS file systems while the underlying issue is investigated.
- Adds runtime OS detection to skip problematic test section on macOS
- Includes detailed TODO comment explaining the reasoning and context
- Maintains full test coverage on Linux systems where the feature is actually used
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
219d1b4 intomainUh oh!
There was an error while loading.Please reload this page.
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.
one nit-pick but otherwise LGTM
| // for our dev container integration. We do not expect the host workspace | ||
| // (where this is used), to ever be run on macOS, as containers are a linux | ||
| // paradigm. | ||
| ifruntime.GOOS!="darwin" { |
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.
nit: maybe fail-fast approach?
ifruntime.GOOS=="darwin" {err=wut.Remove(testFile)require.NoError(t,err,"remove file from watcher failed")return}
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.
Apologies, I hit the green button a little quick.
I can see the benefit to the suggestion, although I think I prefer indenting the flakey code instead as this isn't a permanent change.
Relates tocoder/internal#765
I've been unable to figure out how to fix this (unfortunately my macOS file system knowledge is lacking). Given that this test is causing a lot of flakes at the moment, I think it is best to skip this part of the test on macOS for now.