Fix flaky mtime-ordering bug in CSharpSyntaxTreeCacheTests - #498
Merged
Conversation
GetOrParseFromDisk_re_parses_after_the_file_is_rewritten set the file's last-write-time to two seconds in the future, then called File.WriteAllText -- which itself resets the mtime to the real current time as part of the write, undoing the bump that was supposed to guarantee a distinct timestamp on filesystems with coarse mtime resolution. On an unlucky run the two writes' real timestamps can round to the same tick, so CSharpSyntaxTreeCache wrongly treats the rewritten file as unchanged and returns the stale cached parse -- exactly the failure seen in CI on PR #497 (an unrelated change): https://github.com/reqnroll/Reqnroll.IdeSupport/actions/runs/33029407365/job/98378590600 Swapped the order -- write first, then bump the mtime -- so the deliberate future timestamp actually sticks. Verified stable over 20 repeated runs; full Reqnroll.IdeSupport.LSP.Core.Tests suite still green (684/685, 1 pre-existing skip). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
🤔 What's changed?
GetOrParseFromDisk_re_parses_after_the_file_is_rewrittenset the file's last-write-time two seconds into the future, then calledFile.WriteAllText— which itself resets the mtime to the real current time as part of the write, undoing the deliberate bump the comment claimed would "ensure a distinct last-write-time even on filesystems with coarse mtime resolution." On an unlucky run the two writes' real timestamps can round to the same tick, soCSharpSyntaxTreeCache.GetOrParseFromDiskwrongly treats the rewritten file as unchanged and returns the stale cached parse — failing the test'ssecond.Should().NotBeSameAs(first)assertion.Swapped the order: write the new content first, then bump the mtime afterward, so the future timestamp actually sticks.
⚡️ What's your motivation?
Hit this failing in CI on PR #497, an unrelated change — confirmed the file isn't touched by that PR's diff at all. Traced it to this pre-existing ordering bug in the test itself.
🏷️ What kind of change is this?
🧩 Area(s) touched
src/LSP) — test-only change♻️ Anything particular you want feedback on?
Verified stable over 20 repeated local runs plus the full
Reqnroll.IdeSupport.LSP.Core.Testssuite (684/685, 1 pre-existing unrelated skip). Since the failure was timing-dependent, I can't fully guarantee it'll never recur under sufficiently degenerate CI clock/filesystem conditions, but this closes the concrete ordering bug that caused it.📋 Checklist:
🤖 Generated with Claude Code