Skip to content

Fix flaky WriteWithoutClose/CreateFileWithoutClose functional tests#1959

Merged
tyrielv merged 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/fix-flaky-writewithoutclose
May 6, 2026
Merged

Fix flaky WriteWithoutClose/CreateFileWithoutClose functional tests#1959
tyrielv merged 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/fix-flaky-writewithoutclose

Conversation

@tyrielv
Copy link
Copy Markdown
Contributor

@tyrielv tyrielv commented May 6, 2026

Summary

Fix GC-dependent race condition causing flaky failures in StatusTests.WriteWithoutClose() and StatusTests.CreateFileWithoutClose().

Problem

These tests intentionally open file handles without closing them to validate that git status works with locked files. The handles were created as local variables inside OpenFileAndWriteWithoutClose/CreateFileWithoutClose with no reference kept — making them eligible for GC immediately. If the GC finalized the handles before TearDown ran git reset --hard, the reset would succeed (no lock errors) while the control repo might still have errors, causing a flaky error mismatch in TearDown.

Fix

  • Change FileSystemRunner.CreateFileWithoutClose and OpenFileAndWriteWithoutClose to return IDisposable instead of void
  • SystemIORunner returns the actual FileStream/StreamWriter handle, with an explicit Flush() to ensure data is on disk
  • GitRepoTests helpers return a CompositeDisposable wrapping handles for both GVFS and control repos
  • StatusTests uses using blocks — handles stay alive during the assertion scope and are deterministically disposed before TearDown runs

This eliminates the GC race: both repos have unlocked files when reset --hard runs in TearDown, so errors match (both 0).

Testing

  • Built GVFS.FunctionalTests project successfully (C# compilation clean)
  • The fix is structural — deterministic disposal replaces non-deterministic GC finalization

The StatusTests WriteWithoutClose and CreateFileWithoutClose tests
intentionally open file handles without closing them, then validate
git status behavior. The handles were created as local variables
inside OpenFileAndWriteWithoutClose/CreateFileWithoutClose with no
reference kept — making them eligible for GC immediately. If the
GC collected the handles before TearDown ran git reset --hard, the
reset would succeed (no lock errors) while the control repo might
still have errors, causing a flaky mismatch.

Fix: return IDisposable from the file-handle-leaking methods, hold
them in using blocks in the test methods. Handles stay alive for
the assertion scope and are deterministically disposed before
TearDown runs, eliminating the GC race. Also add StreamWriter.Flush()
to ensure written data is on disk before returning.

Assisted-by: Claude Opus 4.6
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@tyrielv tyrielv marked this pull request as ready for review May 6, 2026 19:39
@tyrielv tyrielv requested a review from KeithIsSleeping May 6, 2026 19:39
@tyrielv tyrielv enabled auto-merge May 6, 2026 19:39
Copy link
Copy Markdown
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@tyrielv tyrielv merged commit 59bfc47 into microsoft:master May 6, 2026
45 checks passed
@mjcheetham mjcheetham mentioned this pull request May 11, 2026
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.

2 participants