Dev CI: UiSmokeWpf lock file pins xunit.v3 4.0.2, and two live test classes skip without DARLING_TEST_PG (#5755)

[erikdarlingdata]

Claude posting for Erik Darling.

Fixes #5755.

#5747 added tools/ui-smoke-wpf with its own packages.lock.json. #5746 then raised xunit.v3 to 4.0.2 in Directory.Packages.props, and its lock-file updates did not include the new project, so locked restore failed with NU1004 on dev. This regenerates that one lock file with dotnet restore --force-evaluate.

The second red on dev is in the Darling whole-tree guard job (CI run 38407981487): 34 failures, all in ExternalAlertDrainLiveTests (21) and ExternalAlertDrainLifecycleLiveTests (13), both added by #5726. Each threw ArgumentNullException (Parameter 'Host'), and so did the scratch-database teardown. That job has no DARLING_TEST_PG, and these two classes read it but never skipped. Each class funnels every live fact through one helper (WithDrainAsync, WithRigAsync), so the fix is one Assert.SkipWhen(string.IsNullOrEmpty(ConnectionString), ...) at the top of each helper, before ScratchPostgres.CreateAsync. The skip leaves before any scratch database exists, so there is no teardown to run on a null connection string. ExternalAlertStoreLiveTests gates the same way, per fact; ExternalAlertGrantLiveTests, ExternalAlertSourceGroupLiveTests and ExternalAlertSourceVerbLiveTests already skip, so nothing else changed. Each fix alone would still leave CI red on the other, so both are here.

Test plan

• dotnet restore PerformanceMonitor.sln --locked-mode --force-evaluate passes locally: exit 0, no error lines, 19 projects restored
• Darling.Tests builds in Debug: 0 warnings, 0 errors
• DARLING_TEST_PG unset, -class filters for the two classes: Total 36, Failed 0, Skipped 34 (the 34 live facts), 2 static facts passed
• DARLING_TEST_PG unset, every *Census* and *Ratchet* class plus LivePostgresCollectionHygieneTests: Total 575, Failed 0, Skipped 1
• Build, WPF smoke and the Darling whole-tree guard job pass in CI (no live PostgreSQL run locally)

Notes

• Lock diff against origin/dev: xunit.v3 4.0.1 to 4.0.2 (direct), its seven xunit.v3.* packages 4.0.1 to 4.0.2, Microsoft.Testing.Platform, Microsoft.Testing.Platform.MSBuild, Microsoft.Testing.Extensions.Telemetry and Microsoft.Testing.Extensions.TrxReport.Abstractions 2.4.0 to 2.5.1, and xunit.analyzers 2.1.0 to 2.2.0. The last five are transitive from xunit.v3 4.0.2. No PerformanceMonitor.* entry changes.
• The same restore also rewrote Lite/packages.lock.json, Darling/PerformanceMonitor.Darling.Viewer/packages.lock.json and deprecated/Dashboard/packages.lock.json. Those changes only move PerformanceMonitor.* project-reference ranges from [3.9.0, ) to [3.10.0, ), and Lite also drops System.Diagnostics.EventLog. They were reverted and are not in this PR. Dev already has them stale; with dev's versions in place, locked restore exits 0 and rewrites them rather than failing.
• A plain --locked-mode run after the forced restore reported "All projects are up-to-date for restore", so the check above adds --force-evaluate to make NuGet re-evaluate each project.

Changelog

SECTION: None
ENTRY: none (CI only)

[claude]

Blind-read confirms the shape this PR describes. Before reading the PR body: Directory.Packages.props:36 already pins xunit.v3 at 4.0.2, and both Lite.Tests/packages.lock.json and Darling/Darling.Tests/packages.lock.json already resolve xunit.v3 to 4.0.2 — only tools/ui-smoke-wpf/packages.lock.json was still at 4.0.1. .github/workflows/build.yml:920 runs dotnet restore tools/ui-smoke-wpf/UiSmokeWpf.csproj --locked-mode, which is exactly the gate LockedModeRestoreCoverageTests.EveryCommittedLockFile_IsReachedByALockedModeRestoreInTheBuildWorkflow requires to exist and would fail NU1004 against the stale lock file. So this is a straggler lock-file regeneration, not a version policy change.

Diffed the full hunk set against the PR body's inventory: xunit.v3 4.0.1→4.0.2 (direct), the matching 4.0.1→4.0.2 bump across its seven xunit.v3.* transitive packages, Microsoft.Testing.Platform/.MSBuild/Extensions.Telemetry/Extensions.TrxReport.Abstractions 2.4.0→2.5.1, xunit.analyzers 2.1.0→2.2.0, and only contentHash/resolved/requested fields touched — no PerformanceMonitor.* entry changes. That matches the body's inventory exactly; nothing else moved in the file.

No REVIEW-TRAPS.md line fits this diff (checked SQL, collectors, analysis, alerting, storage, tests/censuses, workflows, repository-mechanics sections — the closest, the .gitattributes CRLF whole-file-rewrite trap, doesn't apply since the diff is surgical hunks, not every line). Verdict: LGTM.

[claude]

Verdict: LGTM

Claims

• VERIFIED — "#5746 then raised xunit.v3 to 4.0.2 in Directory.Packages.props, and its lock-file updates did not include the new project, so locked restore failed with NU1004 on dev" — Directory.Packages.props:36 pins xunit.v3 at 4.0.2; Lite.Tests/packages.lock.json:14 and Darling/Darling.Tests/packages.lock.json:14 already resolve to 4.0.2 while tools/ui-smoke-wpf/packages.lock.json was still at 4.0.1 pre-PR (per gh pr diff); .github/workflows/build.yml:920 runs a --locked-mode restore of that exact project, which is the check such a mismatch fails.
• VERIFIED — "Lock diff against origin/dev: xunit.v3 4.0.1 to 4.0.2 (direct), its seven xunit.v3.* packages … Microsoft.Testing.Platform, …Telemetry and …TrxReport.Abstractions 2.4.0 to 2.5.1, and xunit.analyzers 2.1.0 to 2.2.0 … No PerformanceMonitor.* entry changes" — confirmed by diffing every changed key/value pair in gh pr diff 5756: only those packages' resolved/requested/contentHash fields move, nothing else.
• UNVERIFIED — "dotnet restore PerformanceMonitor.sln --locked-mode --force-evaluate passes locally: exit 0, no error lines, 19 projects restored" — a live restore result; this is a read-only tree with no .NET SDK invocation performed.
• UNVERIFIED — "Fixes #5755" and the #5747/#5746 PR attribution — the issue/PR history is outside the allowed gh surface for this review (only tools/ui-smoke-wpf/packages.lock.json is in this diff; prior PRs' content wasn't independently checked).

Riskiest lines

1. tools/ui-smoke-wpf/packages.lock.json:18 (xunit.v3 resolved 4.0.2) — the one direct-version bump everything else cascades from — pinned by .github/workflows/build.yml:920's locked-mode restore (NU1004 on any mismatch with Directory.Packages.props).
2. tools/ui-smoke-wpf/packages.lock.json:469 (Microsoft.Testing.Platform resolved 2.5.1, transitive) — pinned the same way, via the same locked-mode restore walking the full graph.
3. tools/ui-smoke-wpf/packages.lock.json contentHash fields throughout — NuGet-computed; a wrong hash fails the same restore rather than silently installing a mismatched package.

Traps near this diff

• Repository mechanics: ".gitattributes says text=auto eol=crlf: … A diff that touches every line of a file is a line-ending conversion until proven otherwise" — does not apply: the diff is surgical hunks around the changed package entries (confirmed via gh pr diff), not a whole-file rewrite.
• No other REVIEW-TRAPS.md line (SQL, collectors/deltas, analysis, alerting, storage/MCP, tests/censuses, workflows/CI) is near a lock-file-only dependency bump; none apply.

Divergence (blind read vs PR body)

None — the body's account matches the diff on every claim graded above, including its explicit scoping note that the other three lock files the same restore rewrote (Lite, Darling Viewer, deprecated Dashboard) were reverted and deliberately excluded from this PR; gh pr diff --name-only confirms only tools/ui-smoke-wpf/packages.lock.json is in this diff.

Reviewer checklist

• Confirm CI's build job restore step (.github/workflows/build.yml:920) and the wpf-smoke workflow both go green on this PR — the body's local restore result and the unchecked "Build and WPF smoke pass in CI" box are both live results this read-only review couldn't reproduce.

[claude]

Follow-up pass, scoped to c1594bef4..7571099 (the two test-file commits fixing #5755's second red).

Blind read: both WithRigAsync (ExternalAlertDrainLifecycleLiveTests.cs:454) and WithDrainAsync (ExternalAlertDrainLiveTests.cs:501) are the single funnel every [Fact] in each class awaits directly, and the new Assert.SkipWhen(string.IsNullOrEmpty(ConnectionString), …) line in each lands before the respective ScratchPostgres.CreateAsync call — so a null/empty DARLING_TEST_PG now skips before any scratch database (or its teardown) exists, exactly as the comment says. This matches the established idiom used elsewhere in the suite verbatim (e.g. StorageGrowthNoHistoryLiveTests.cs:37, ServerEditLiveTests.cs:96, and the nine call sites already gated this way in ExternalAlertStoreLiveTests.cs).

Checked the PR body's claim that ExternalAlertGrantLiveTests, ExternalAlertSourceGroupLiveTests and ExternalAlertSourceVerbLiveTests "already skip" — confirmed, all three already have Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), …) guards per fact, so nothing else needed changing there.

No REVIEW-TRAPS.md line applies to this range (checked tests/censuses and repository-mechanics sections specifically): this isn't a whole-file CRLF rewrite (surgical 4-line hunks per file), and ExternalAlertDrain has no Lite counterpart (find . -iname "*ExternalAlertDrain*" returns only Darling paths), so the Lite/Darling parity trap doesn't apply either.

[claude]

Verdict: LGTM

Claims

• VERIFIED — "Each class funnels every live fact through one helper (WithDrainAsync, WithRigAsync), so the fix is one Assert.SkipWhen(...) at the top of each helper, before ScratchPostgres.CreateAsync" — confirmed at ExternalAlertDrainLifecycleLiveTests.cs:456-461 and ExternalAlertDrainLiveTests.cs:503-508; every [Fact] in both classes calls WithRigAsync/WithDrainAsync directly (12 and 16 call sites respectively), so one guard covers every live fact in each class.
• VERIFIED — "The skip leaves before any scratch database exists, so there is no teardown to run on a null connection string" — the new Assert.SkipWhen line precedes the await ScratchPostgres.CreateAsync(ConnectionString!, ct) call in both helpers; a thrown SkipException short-circuits before the try/finally teardown block is even entered.
• VERIFIED — "ExternalAlertGrantLiveTests, ExternalAlertSourceGroupLiveTests and ExternalAlertSourceVerbLiveTests already skip, so nothing else changed" — all three already carry Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), …) guards per fact (confirmed via grep); this PR's diff touches only the two previously-ungated classes.
• UNVERIFIED — "DARLING_TEST_PG unset, -class filters for the two classes: Total 36, Failed 0, Skipped 34 (the 34 live facts), 2 static facts passed" — a live dotnet test run result; not reproducible from a read-only tree.

Riskiest lines

1. Darling/Darling.Tests/ExternalAlertDrainLifecycleLiveTests.cs:458 — the skip guard itself; if misplaced after CreateAsync it would reintroduce the ArgumentNullException on teardown — pinned by CI's Darling whole-tree guard job (no DARLING_TEST_PG) rerunning on this PR.
2. Darling/Darling.Tests/ExternalAlertDrainLiveTests.cs:506 — same shape, same risk, second class — pinned the same way.
3. Both guards rely on ConnectionString reading Environment.GetEnvironmentVariable("DARLING_TEST_PG") correctly (ExternalAlertDrainLiveTests.cs:40, ExternalAlertDrainLifecycleLiveTests.cs:43) — unchanged by this PR, so UNPINNED only in the sense that no unit test asserts the getter itself, but it's identical to the already-working pattern in eight other live-test classes.

Traps near this diff

• Repository mechanics: "Lite and Darling ship in parity … a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding" — does not apply: ExternalAlertDrain and its live tests exist only under Darling/ (find . -iname "*ExternalAlertDrain*" returns no Lite paths), so there is no Lite counterpart to update.
• Tests/censuses: ".gitattributes … A diff that touches every line of a file is a line-ending conversion until proven otherwise" — does not apply: both hunks are surgical 4-line insertions, not whole-file rewrites.

Divergence (blind read vs PR body)

None — the body's account of this range (one SkipWhen per helper, placed before scratch-database creation, with the three other classes already gated) matches the diff and the surrounding code exactly.

Reviewer checklist

• Confirm the Darling whole-tree guard job (CI, no DARLING_TEST_PG) now shows 34 skips and 0 failures for these two classes, as the PR body's local -class-filtered run claims.