Skip to content

Conversation

ilevkivskyi
Copy link
Member

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.

@ilevkivskyi
Copy link
Member Author

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)
Copy link
Member

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?

Suggested change
fudge_dir_mtimes(WORKDIR, -1)

Copy link
Member Author

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.

Copy link
Collaborator

@msullivan msullivan left a 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?

@ilevkivskyi
Copy link
Member Author

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.

@ilevkivskyi ilevkivskyi merged commit a3e488d into python:master Nov 19, 2023
@ilevkivskyi ilevkivskyi deleted the fix-macos branch November 19, 2023 17:33
@JukkaL
Copy link
Collaborator

JukkaL commented Nov 20, 2023

Thanks for looking into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mypyc runtime tests with py39-macos is unstable
4 participants