-
-
Notifications
You must be signed in to change notification settings - Fork 2.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
An attempt to fix mypyc tests on MacOS #16520
Conversation
OK, another 20 runs passed locally, and a second re-run also passed in GHA. I think we can merge this unless there are objections. |
@@ -172,12 +172,10 @@ def run_case_inner(self, testcase: DataDrivenTestCase) -> None: | |||
# new by distutils, shift the mtime of all of the | |||
# generated artifacts back by a second. | |||
fudge_dir_mtimes(WORKDIR, -1) |
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.
If we're just doing time.sleep()
on all platforms now, maybe we should just delete this line (and adjust the comment above it)? Or comment it out?
fudge_dir_mtimes(WORKDIR, -1) |
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.
There is no harm in doing both. I would just keep it as is, and hope someone will find a better solution soon.
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.
Sure, need to stop bleeding.
But maybe something like having fudge_dir_mtimes try to wait for the change to become visible could work?
Hm, I just tried that (in couple variations), and somehow it still doesn't work reliably. Maybe there is some weird rounding behavior. I give up for now. |
Thanks for looking into this! |
Fixes #16420
Although this is not 100% clear yet, but after 20 runs on a Mac I have it no longer fails (without this patch it failed 20% of times). Btw, contrary to the comment, my Linux Mint (which is an Ubuntu derivative) works perfectly (i.e. test passed 20 times even after I removed the
sleep()
). So it is not really Mac vs Linux issue.