HealthTests: diagnostic output for flaky test investigation#1954
Closed
tyrielv wants to merge 12 commits into
Closed
HealthTests: diagnostic output for flaky test investigation#1954tyrielv wants to merge 12 commits into
tyrielv wants to merge 12 commits into
Conversation
Retarget from net471 to net10.0-windows10.0.17763.0 across all managed projects. Enable NativeAOT self-contained deployment, eliminating the .NET runtime dependency. Build infrastructure: - global.json: pin SDK 10.0.203 - Directory.Build.props: centralized TFM, SelfContained, PublishAot, OptimizationPreference=Speed - Directory.Build.targets: AOT build targets; opt out test projects and GVFS.MSBuild (netstandard2.0) from AOT - Build.bat: 3-step build (dotnet restore, VS MSBuild for C++, dotnet publish for managed AOT binaries) - publish-aot.ps1: standalone script for local AOT publish testing (CI uses Build.bat; this script is for dev iteration) - Update output paths in all scripts (net471 -> net10.0-.../publish) - Update CI to .NET 10 SDK and windows-2025 runner - Update installer MinVersion to 10.0.17763 Package updates: - Microsoft.Windows.ProjFS 1.1 -> 2.1.0: pure C# P/Invoke replacing C++/CLI interop, required for NativeAOT compatibility - Microsoft.Data.Sqlite 2.2.4 -> 9.0.4, Microsoft.Build.* 16 -> 17.12.6 - Add System.Diagnostics.EventLog, System.IO.Pipes.AccessControl: previously included in .NET Framework, now separate packages - Remove GVFS.ProjFS (ProjFS is now a Windows OS feature) Unit test fixture updates for new ProjFS managed API surface. Output: ~20 MB native GVFS.exe, 36.7 MB installer (vs 107 MB with full self-contained runtime) Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Assembly.Location returns empty string under NativeAOT since there is no managed assembly on disk. Assembly.GetName().Version returns null. - ProcessHelper: use Environment.ProcessPath with null guard (can be null in certain hosting scenarios), fall back to AppContext.BaseDirectory - HooksInstaller: same Environment.ProcessPath pattern with null guard - GVFSEnlistment: AppDomain.CurrentDomain.FriendlyName replaces Assembly.GetEntryAssembly().GetName() for process name - JsonTracer/PrettyConsoleEventListener: same pattern for version string Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
NamedPipeServerStream (WindowsPlatform.cs): ACL-accepting constructor removed from .NET Core; use NamedPipeServerStreamAcl.Create extension method. Directory ACL APIs (WindowsFileSystem.cs, GVFSService.Windows.cs): Static Directory.GetAccessControl/SetAccessControl and Directory.CreateDirectory(path, security) removed from .NET Core; replaced with DirectoryInfo instance methods and DirectorySecurity.CreateDirectory extension. Uri escaping (CloneVerb.cs, GVFSVerb.cs, OrgInfoApiClient.cs): Uri.EscapeUriString obsoleted in .NET 10 (does not escape '#', '?'); use Uri.EscapeDataString. HttpUtility.UrlEncode (System.Web) replaced with WebUtility.UrlEncode (System.Net). UseShellExecute (WindowsPlatform.cs, InProcessMount.cs): .NET Framework defaults UseShellExecute=true (ShellExecuteEx, no handle inheritance). .NET 10 defaults to false (CreateProcess, handles inherited). Without this, GVFS.Mount.exe inherits the caller's stdout pipe handle, causing callers that read to EOF to block indefinitely. Truncated loose object detection (GitRepo.cs): .NET 10 DeflateStream silently returns partial data on truncated zlib instead of throwing InvalidDataException. CountingStream wrapper compares actual bytes read to header-declared size to detect corruption. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
System.Management requires COM interop which is incompatible with NativeAOT. Replace WMI queries (MSFT_Volume, MSFT_Partition, MSFT_Disk, MSFT_PhysicalDisk) with direct kernel32 DeviceIoControl calls using IOCTL_STORAGE_QUERY_PROPERTY and IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS for disk telemetry collection. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
ProjFS managed API v2.1.0 uses Marshal.PtrToStringUni which returns null for IntPtr.Zero (kernel operations with PID 0). The old C++/CLI wrapper returned String.Empty. Null-coalesce to match old behavior in all three callback sites (OnPlaceholderFileCreated, OnPlaceholderFolderCreated, OnPlaceholderFileHydrated); ConcurrentDictionary does not accept null keys. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Replace HttpClientHandler with SocketsHttpHandler for explicit connection pool lifecycle management: configurable MaxConnectionsPerServer (2x CPU count), PooledConnectionLifetime, and PooledConnectionIdleTimeout. Remove UseDefaultCredentials (not supported on SocketsHttpHandler) and ServicePointManager usage (.NET Framework only). GitSsl: X509Certificate2(byte[]) constructor obsoleted; use X509CertificateLoader.LoadCertificate. GitAuthentication: adapt credential flow for new HTTP handler. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
NativeAOT cannot use runtime reflection for JSON serialization. GVFSJsonContext provides source-generated System.Text.Json serializers for 25+ types used in named pipe messages and configuration. GVFSJsonOptions chains source-gen (primary) with reflection fallback for types not yet in the context, allowing incremental migration. NamedPipeMessages: add parameterless constructors required by the source generator's deserialization codegen. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
.NET 10's FileInfo property setters no longer open write handles that trigger ProjFS placeholder hydration. Adapt tests that relied on this. BasicFileSystemTests: replace ExpandedFileAttributesAreUpdated with two focused tests: - PlaceholderMetadataSurvivesHydration: sets timestamps + Hidden on a placeholder, verifies they took effect, hydrates via read+write, and asserts CreationTime and Hidden survived the conversion. - HydratedFileTimestampsAndAttributesAreUpdated: hydrates first, then sets all properties and verifies they stick. GitCommandsTests: ChangeTimestampAndDiff now explicitly hydrates via read+write before adjusting timestamps, since File.SetLastWriteTime no longer triggers ProjFS hydration. GVFSProcess: add 5-minute timeout per gvfs process invocation to prevent CI hangs. Stream stdout/stderr for real-time CI output. functional-tests.yaml: reduce mount sleep from 500ms to 100ms, add timeout-minutes and --workers=1 for sequential execution. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Capture raw 'gvfs health' output and validate line count before index-based parsing. On failure, the full output is included in the assertion message to reveal what gvfs health actually returned. Also restrict CI functional test matrix to slice 7 only (where HealthTests lives) for faster iteration during investigation. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Adds a 'copy' matrix dimension so slice 7 runs 5 times per config/arch combination (20 total functional test jobs). Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Flake didn't reproduce with slice 7 in isolation. Reverting to full matrix since the failure reportedly requires the full CI load to manifest. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Process.WaitForExit(timeout) does not wait for async output streams (BeginOutputReadLine/BeginErrorReadLine) to finish. Call parameterless WaitForExit() after the timed overload to drain the remaining pipe data. This is the root cause of HealthTests flakiness - gvfs health output was being read before stdout was fully flushed, causing the test to see fewer lines than expected. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Contributor
Author
|
Root cause found: stdout truncation in WaitForExit(timeout). Fix applied to net10-pr (#1953) instead. |
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.
Problem
HealthTests.PlaceholdersChangeHealthScores and ModifiedPathsChangeHealthScores fail intermittently in CI with ArgumentOutOfRangeException — the test parses gvfs health output by line index without validating the output first.
Changes
gvfs healthoutput and validate line count before index-based parsingContext
Observed in run 25186133967 on branch tyrielv/net10-pr (3 of 4 failed jobs).
ADO: #62098957