The migration-lock test does not hang - #100
Merged
Merged
Conversation
ci.yml justified its -run filter by saying the mongo suite contains a hanging migration-lock test. TestMigrate_DoesNotBreakFreshLock does not hang. Against a single-node rs0 replica set matching this job's own service it passes in 11 to 12 seconds over four runs, waiting out the migration lock-acquire retry rather than blocking on it. The claim was stale, and stale comments are worse than absent ones because they get believed. This one did: I repeated it as fact when writing up a task for another session, which sent somebody looking for a hang that was not there. They found the truth by running it. I have now run it myself rather than take their number on trust, which is the same courtesy. Comment only. The filter itself is untouched, deliberately. Its stated justification being wrong does not tell you what the right filter is, and the real question underneath, whether the other 89 integration-tagged tests belong in this job, deserves a decision rather than being resolved as a side effect of a comment fix. The comment now says that plainly and says to run them as a set against main before widening anything, because the only count taken of them so far was taken against a 300-commit-stale tree. One em dash removed as a side effect of rewriting the block. The one left at line 199 is in an unrelated comment and stays there rather than widening this diff.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ci.ymljustified its-runfilter by saying the mongo suite contains a hanging migration-lock test. It does not.TestMigrate_DoesNotBreakFreshLockpasses in 11 to 12 seconds over four runs, against a single-node rs0 replica set matching this job's own service. It waits out the migration lock-acquire retry rather than blocking on it.Why this is worth a PR on its own
Stale comments are worse than absent ones, because they get believed. This one did. I repeated it as fact when writing up a task for another session, which sent somebody looking for a hang that was never there. They found the truth by running it, and I have now run it myself rather than take their number on trust.
That is the whole change: comment only, no behaviour.
What it deliberately does not do
The filter itself is untouched. Its stated justification being wrong does not tell you what the right filter is.
The question underneath is real and unresolved: there are 96 integration-tagged test functions, this pattern matches 7, and the remaining 89 are built by nothing and run by nothing while sitting in the tree looking like coverage. Widening the filter is a decision with a cost either way, and resolving it as a side effect of a comment fix would be the wrong way to make it. The comment now states the numbers and says to run those 89 as a set against main before changing anything, because the only count taken of them so far was taken against a tree 300 commits behind and came out wrong.
Verification
Four runs, 11.38s, 12.36s, 11.60s, 11.08s, all passing, no intermittency. YAML still parses, all seven jobs intact, and every changed line is a comment.
One em dash disappeared as a side effect of rewriting the block. The one remaining at line 199 is in an unrelated comment and stays, rather than widening this diff to cover prose I am not otherwise touching.