Repository navigation
[pull] master from microsoft:master - #146
Merged
Merged
Conversation
When a functional test fails because its GVFS.Mount is unreachable (a hang or silent exit), we currently have no post-mortem data. Per-test enlistment inlined into the console by TestResultsHelper.OutputGVFSLogs -- lossy under parallel fixtures and empty when the mount hung. There is no process dump, so a mount deadlock leaves zero diagnostic signal. On test failure only, into a CI-uploadable diagnostics directory: GVFSFunctionalTestEnlistment.CaptureFailureDiagnostics runs first in DeleteEnlistment, gated on TestStatus.Failed, before the enlistment directory is deleted. The mount-process PID discovery is extracted from KillMountProcess into a shared GetMountProcessIds helper. CI: functional-tests.yaml sets GVFS_TEST_DIAGNOSTICS_DIR and uploads it as a FailureDiagnostics artifact with if: always(). Review follow-ups: - GetMountProcessIds doubled the backslashes in the enlistment path before using it in a PowerShell -like wildcard. In -like, '\' is a literal (not an escape), so the doubled pattern never matched a real single-backslash command line: CaptureFailureDiagnostics found no live mount and wrote no minidump, and KillMountProcess silently killed nothing. Match on the enlistment's unique leaf folder id instead -- present on the GVFS.Mount command line (launched with PrimaryEnlistmentRoot), unique, and free of path separators or wildcard metacharacters, so it needs no escaping. - WaitForExit(int) only guarantees the helper process has exited -- it does not guarantee the async OutputDataReceived callbacks (raised on the thread pool as data arrives) have all run yet. Reading the output buffer immediately afterward could race the last callback and intermittently drop a trailing PID, making GetMountProcessIds miss a live mount process. Call the parameterless WaitForExit() right after the timed wait succeeds to drain any pending async output callbacks before parsing. Assisted-by: Claude Sonnet 5 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
GetDirectoryEnumeration logs "Failed to find active enumeration ID" when an enumeration ID is absent. Every non-eviction miss carried the reason Unknown, which hid two different causes: - EndedRecently: ProjFS delivered a Get that raced or followed the End for the same enumeration (a benign kernel close/query race). - NeverSeen: GVFS never held the ID (it never started, or it predates a provider restart). The classification, the once-per-ID error de-duplication, and the bounded tracking maps are extracted into a new EnumerationFailureTracker class (mirroring the MissingTreeTracker pattern) so the policy is cohesive and unit testable on its own. The tracker owns all three "why is this ID absent" maps - recently evicted, recently ended, and recently reported - and exposes RecordEvicted, RecordEnded, ClassifyMiss, and TryReserveReport. The virtualizer records an end (and an eviction) before removing the ID from activeEnumerations, so a racing Get always finds the ID in one collection or the other; ClassifyMiss attributes Evicted (most actionable), then EndedRecently, then NeverSeen. The old Unknown value is renamed NeverSeen. When an End removes nothing from the active set and the ID was not evicted, GVFS never actually held it, so the ended marker recorded before the removal is undone (UndoEnded) - keeping the record-before-remove ordering for the normal path while avoiding a later Get being skewed to EndedRecently for an ID that was never seen. De-duplicate the error: a caller that re-enumerates a lost handle can emit the same error a very large number of times on one machine. The tracker emits the full error once per ID within a window; the first occurrence still logs at Error, so the machine-based signal stays intact. The returned HResult does not change. The tracker prunes its maps on a throttle from every record point, so no map can grow unbounded when one callback (e.g. End) stops arriving - the never-ended scenario this instrumentation targets. It keeps lock-free ConcurrentDictionary state; a coarse lock would serialize the hot enumeration path. The EnumerationFailureReason values are a case-sensitive contract consumed by the release-readiness telemetry dashboard; its cause bucketing must add EndedRecently and NeverSeen. Tests: EnumerationFailureTracker is unit-tested directly (classification, Evicted-over-EndedRecently precedence, eviction undo, ended undo, dedup, prune/retention); the virtualizer tests cover the wiring, the record-before-remove ordering, the Evicted-over-EndedRecently precedence, and that an End for a never-held ID does not skew a later Get to EndedRecently. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
The GitCommands functional tests compare a GVFS repo against a plain "control"
git repo. Every control repo fetches from one machine-global bare cache. The
cache was set up in a static constructor that checked Directory.Exists and then
either cloned or fetched, and it discarded every git exit code.
That setup was not atomic and not verified. Fixtures run in parallel and the
cache path is shared by concurrent test processes on the same machine, so a
process could observe a half-built clone directory (Directory.Exists is true
before the clone finishes) and fetch from an incomplete repo. A transient
clone or fetch failure had the same effect. The cache was then left missing
branches, the failure was swallowed, and every GitCommands test failed its
setup checkout with:
error: pathspec 'FunctionalTests/20201014' did not match any file(s) known to git
A local repro confirmed the cause: against a healthy cache 0/50 control-repo
builds fail; against a cache missing the branch 50/50 fail with that exact
error.
Make the setup robust:
- Serialize cache creation and refresh across processes with a system-wide
mutex.
- Build the cache atomically: clone into a temporary directory, verify the
base branch is present, then move it into place so no other process sees a
partial cache.
- Retry transient clone and fetch failures, and rebuild the cache when
verification fails.
- Fail loudly with a clear message when a control repo cannot fetch or check
out its branch, and retry the whole control-repo build, instead of producing
a broken repo that fails 20+ tests with a confusing cascade.
With the fix, a control repo that starts from a broken cache self-heals and
0/50 builds fail.
Assisted-by: Claude Opus 4.8
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
The previous commit rebuilt the shared control-repo cache with "git clone
--bare". A --bare clone copies only refs/heads/* and tags. Some tests fetch
commits by SHA that live outside refs/heads (for example RebaseTests fetches
the tip of FunctionalTests/RebaseTestsSource_20170130). Those objects were
absent from a --bare cache, so the control repo's fetch failed with:
fatal: git upload-pack: not our ref <sha>
and, now that Fetch throws on a non-zero exit, RebaseSmallOneFileConflict
failed on functional-test slice 4 (both architectures).
Rebuild the cache with "git clone --mirror" instead. --mirror maps
refs/*:refs/*, so the cache carries the complete ref set, matching the
refresh path's "fetch origin +refs/*:refs/*". A local test confirms a --bare
clone omits a non-refs/heads ref while a --mirror clone retains it.
Assisted-by: Claude Opus 4.8
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
The prior commits regressed functional-test slice 4: RebaseTests fetch a specific commit by SHA (for example 5d29951...), and those fetches failed with "fatal: git upload-pack: not our ref". Two problems caused this. 1. The shared cache is machine-global and persistent on CI runners. It can hold commits reachable only from branches that upstream no longer advertises, which some tests still fetch by SHA. My EnsureSharedCache could rebuild (replace) that cache when a branch check or refresh looked wrong, which drops those commits. Decide fresh-build vs refresh by Directory.Exists (matching the original code), refresh an existing cache best-effort, and never delete or rebuild it. 2. upload-pack refuses to serve a SHA that is not an advertised ref tip unless the served repo allows it. Enable uploadpack.allowAnySHA1InWant (plus reachable and tip) on the cache so control repos can fetch any commit present in the cache by SHA. Also make ControlGitRepo.Fetch tolerant again: it retries, but on final failure it logs instead of throwing. Whether the cache can serve a given SHA is a property of the cache, not the test; the test's own ValidateGitCommand (control repo vs GVFS repo) remains the correctness gate. The loud failure for a genuinely broken cache stays on the base-branch checkout in Initialize, which is what caused the original swallowed cascade. Fresh caches are still built with clone --mirror (complete ref set) into a temporary directory and moved into place under a system-wide mutex, so no process observes a half-built cache. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Capture mount dumps and preserve logs when a functional test fails
FunctionalTests: make shared control-repo cache setup resilient
Split enumeration-miss cause + extract EnumerationFailureTracker + de-duplicate error telemetry
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )