-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Several of the FileSystemWatcherTests are failing after PR 14043 #9000
Comments
FYI. @stephentoub, @pjanotti, @JeremyKuhne, since you were the reviewers and/or tagged for review on the associated PR. |
The ftruncate call fires a MODIFY inotify event (even on an empty file). I think the solutions are:
Option 4. incurs an overhead of an extra syscall. Option 3. is complicated by the fact that the tests in tests/FileSystemWatcher.cs creates the file through TempFile which in turn calls File.WriteAllBytes. |
Thanks @tannergooding. @poizan42 at first glance I agree: either solution 1 or 2, initially I am leaning towards 1. |
/cc @jkotas Marked failed tests as active issues: dotnet/corefx#24305 |
@pjanotti @stephentoub I can make a PR, but first I would like to know which of the solutions to go with. The strict verification on non-OSX ensures that backwards compatibility is kept at least per-platform, even though user programs probably shouldn't rely on that. But of course dotnet/coreclr#14043 already did change the behaviour so it could break badly written programs. |
Thanks @poizan42. The less strict verification makes more sense to me, making it very strict means that we risk to have to adapt whenever a new platform comes onboard and the adaptation is to "correct" the test. |
Hi @poizan42 do you have time to work on this? If yes I can assign it to you, if not, (of course) it is fine, I can take a look at it. |
@pjanotti You can assign me :) |
… platforms. Fixes dotnet/coreclr#14154.
… platforms. Fixes dotnet/coreclr#14154. (#24406)
Ah, that makes sense. So let's do that on your next contribution to coreclr. |
@poizan42 yes, fixed - check your inbox for the invitation. |
… platforms. Fixes dotnet/coreclr#14154. (dotnet#24406)
It seems that dotnet/coreclr#14043 introduced four failures in the CoreFX baseline tests:
These were first detected in dotnet/coreclr#14119 (Job: https://ci.dot.net/job/dotnet_coreclr/job/master/job/jitstress/job/x64_checked_ubuntu_corefx_baseline_prtest/98/). But they also appeared earlier in the daily runs of the job: https://ci.dot.net/job/dotnet_coreclr/job/master/job/jitstress/job/x64_checked_ubuntu_corefx_baseline/298/.
NOTE: It wasn't detected earlier due to other build failures in the job that was caused by an unrelated change.
I did validate that it repro's as of the PR commit (555f436), but not in the commit prior to that (5d37885).
The text was updated successfully, but these errors were encountered: