-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Disable even more FileWatching tests #46497
Conversation
Seems to still fail in FileWatching on mac |
2ce9274
to
393bd6a
Compare
393bd6a
to
24ba685
Compare
75b554a
to
5d15b9f
Compare
stdlib/FileWatching/test/runtests.jl
Outdated
@@ -32,7 +35,9 @@ for i in 1:n | |||
if !fd_in_limits && Sys.islinux() | |||
run(`ls -la /proc/$(getpid())/fd`) | |||
end | |||
@test fd_in_limits | |||
if ismacos_arm |
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.
Should this be !ismacos_arm
? Does this test really only work on macOS ARM?
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.
Yeah, that was a typo. :-/
if !ismacos_arm | ||
@test fd_in_limits | ||
end |
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.
Can also use
@test fd_in_limits skip=ismacos_arm
which marks the test as broken, which I guess is a useful reminder
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.
Those tests are probably never going to be reliable on macos anyhow, I guess setting them as broken is cleaner. Not sure
8ce124f
to
3fd65e0
Compare
I'm not particularly happy with the x86 vs aarch64 difference. These are mac kernel differences, not architecture specific issues. Either we expect the macos system to have these invariants or not. It shouldn't be platform specific. |
These tests are also unreliable so disable them too for now. See #46185