-
Notifications
You must be signed in to change notification settings - Fork 348
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
tests: fix mtime flakiness in git gc tests #3539
Conversation
I just blindly ran
Maybe 1 second isn't enough? I haven't spent time looking at the test since I figured you'd have a better idea anyway :) |
Ok, this appears to be an mtime problem in gix. |
Updated tests to not rely on in-memory cache. I've tested the change on old ext3 image (of 1sec timestamp accuracy) where the problem can be ~100% reproduced. |
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.
Thanks for fixing.
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.
My test of 20 runs went through fine, thanks a lot!
Apparently, these gc() invocations rely on that the previous "git gc" packed all refs so there are no loose refs to compare mtimes. If there were new (or remaining) loose refs, mtime comparison could fail. I also added +1sec to effectively turn off the keep_newer option, which isn't important in these tests.
This addresses the test instability. The underlying problem still exists, but it's unlikely to trigger user-facing issues because of that. A repo instance won't be reused after gc() call. Fixes jj-vcs#3537
#3537
@thoughtpolice can you try this patch? I cannot reproduce the problem reliably at my end.
Checklist
If applicable:
CHANGELOG.md