Skip to content

Close race condition in Filewatching tests #46028

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

Merged
merged 1 commit into from
Jul 15, 2022

Conversation

staticfloat
Copy link
Member

On some machines, we are able to delay starting our loop until more than
a second after we take the lock, which of course breaks the test. Let's
use a synchronization barrier to ensure that we're testing what we
intend to.

@giordano
Copy link
Contributor

Does this fix #45982?

Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

It should use a Base.Event though

@DilumAluthge DilumAluthge added the system:apple silicon Affects Apple Silicon only (Darwin/ARM64) - e.g. M1 and other M-series chips label Jul 13, 2022
@DilumAluthge
Copy link
Member

DilumAluthge commented Jul 13, 2022

Should these changes also be upstreamed to the Pidfile.jl package?

On some machines, we are able to delay starting our loop until more than
a second after we take the lock, which of course breaks the test.  Let's
use a synchronization barrier to ensure that we're testing what we
intend to.
@staticfloat staticfloat force-pushed the sf/filewatching_test_fix branch from d81be11 to 3e18669 Compare July 13, 2022 21:37
@staticfloat
Copy link
Member Author

Waiting for aarch64-apple-darwin test

@staticfloat staticfloat merged commit c7e2f9b into master Jul 15, 2022
@staticfloat staticfloat deleted the sf/filewatching_test_fix branch July 15, 2022 17:46
@giordano
Copy link
Contributor

@staticfloat

Does this fix #45982?

Bump

@staticfloat
Copy link
Member Author

Yep, sorry about that

@giordano giordano linked an issue Jul 16, 2022 that may be closed by this pull request
ffucci pushed a commit to ffucci/julia that referenced this pull request Aug 11, 2022
On some machines, we are able to delay starting our loop until more than
a second after we take the lock, which of course breaks the test.  Let's
use a synchronization barrier to ensure that we're testing what we
intend to.
pcjentsch pushed a commit to pcjentsch/julia that referenced this pull request Aug 18, 2022
On some machines, we are able to delay starting our loop until more than
a second after we take the lock, which of course breaks the test.  Let's
use a synchronization barrier to ensure that we're testing what we
intend to.
Keno added a commit that referenced this pull request Aug 28, 2022
I think the synchronization in #46028 was incomplete to actually fix
the issue. Try to fix it by accounting the time taken to wait for
the synchronization event into the lock time. Otherwise, the test
pass/fail depends on the scheduler ordering.
Keno added a commit that referenced this pull request Aug 28, 2022
I think the synchronization in #46028 was incomplete to actually fix
the issue. Try to fix it by accounting the time taken to wait for
the synchronization event into the lock time. Otherwise, the test
pass/fail depends on the scheduler ordering.
Keno added a commit that referenced this pull request Aug 29, 2022
I think the synchronization in #46028 was incomplete to actually fix
the issue. Try to fix it by accounting the time taken to wait for
the synchronization event into the lock time. Otherwise, the test
pass/fail depends on the scheduler ordering.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
system:apple silicon Affects Apple Silicon only (Darwin/ARM64) - e.g. M1 and other M-series chips
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Intermittent test failure in FileWatching
4 participants