From e2e84354623b442e8c4ab24ac41db15412feb7c2 Mon Sep 17 00:00:00 2001 From: Shane Neuville <5375137+PureWeen@users.noreply.github.com> Date: Thu, 18 Jun 2026 12:59:02 -0500 Subject: [PATCH 1/3] Deflake release-readiness recency test: remove wall-clock-dependent live assertions, add deterministic fixture coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The release-readiness skill's own test suite contained a time-bomb: four live, wall-clock-dependent assertions hardcoded hasRecentActivity = $true against real release branches (SR8, SR9, the active-SR foreach loop, and preview6). hasRecentActivity is computed by the detector as `git log --since=7.days` > 0, so it is only true while the real branch has had a commit in the last 7 days. The moment a servicing branch goes quiet for 7 days — a NORMAL end-of-cycle state — the assertion flips red. On 2026-06-18 this happened: SR8's last commit (the SR7->SR8 merge #35810) landed 2026-06-11, so the 7-day window returned 0 and the two SR8 assertions went red. They self-heal on the next commit, but that is a credibility bug in a suite whose entire value proposition is determinism. This change: - Removes the wall-clock-dependent hardcoded-$true live assertions. The end-to-end detector run now asserts only that hasRecentActivity is a real [bool] the detector emitted, never a date-dependent value. - Adds genuinely deterministic coverage of the recency-window math via a synthetic fixture: a throwaway temp git repo with commits at controlled dates (GIT_AUTHOR_DATE/GIT_COMMITTER_DATE), then calls the REAL Get-RecentCommitCount (dot-sourced) against it and asserts exact counts for 7/10/60/1-day windows plus the origin/ ref form. Zero network, zero dependence on "today"; the temp repo is cleaned up in a finally. - Corrects the misleading comments that equated "active SR" with hasRecentActivity = true; an active SR can legitimately idle >7 days. Follow-up to #35971. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../tests/Test-ReleaseReadiness.ps1 | 90 +++++++++++++++++-- 1 file changed, 82 insertions(+), 8 deletions(-) diff --git a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 index e86be0bb7f18..3b48645d18ea 100644 --- a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 @@ -531,6 +531,62 @@ if (-not (Test-Path $detectScriptPath)) { # Empty shipped set: every preview is in-flight. Assert-Eq -Label "no shipped previews: preview1 in-flight" -Expected $true -Actual (Test-IsPreviewBranchInFlight -PreviewNumber 1 -ShippedPreviews $emptyPreviewSet) Assert-Eq -Label "no shipped previews: preview20 in-flight" -Expected $true -Actual (Test-IsPreviewBranchInFlight -PreviewNumber 20 -ShippedPreviews $emptyPreviewSet) + + # ─────────── Get-RecentCommitCount: deterministic recency-window coverage ─────────── + # The detector derives every tracker's `hasRecentActivity` from + # Get-RecentCommitCount (git log --since=.days). The live E2E + # assertions deliberately DON'T pin that flag's value — it's wall-clock + # dependent: a servicing branch idle for >Days flips it to $false, which is a + # NORMAL end-of-cycle state, not a bug. Prove the window math itself is correct + # HERE instead, against a throwaway repo whose commits have controlled dates. + # This is fully deterministic: zero network, zero dependence on "today". + Write-Host "`n[Unit] Get-RecentCommitCount recency window (synthetic fixture)" -ForegroundColor Cyan + $savedRepo = $Repo + $fixtureRepo = Join-Path ([System.IO.Path]::GetTempPath()) "rr-recency-fixture-$([guid]::NewGuid().ToString('N'))" + try { + New-Item -ItemType Directory -Path $fixtureRepo -Force | Out-Null + git -C $fixtureRepo init -q 2>&1 | Out-Null + git -C $fixtureRepo config user.email 'rr-test@example.com' 2>&1 | Out-Null + git -C $fixtureRepo config user.name 'RR Test' 2>&1 | Out-Null + + # Three commits at known ages relative to "now". The 1-day margins on either + # side of the 7-day window keep every assertion robust (no boundary fuzz). + $now = Get-Date + foreach ($c in @( + @{ Msg = 'c30'; Age = 30 } # well outside any window under test + @{ Msg = 'c8'; Age = 8 } # just OUTSIDE the 7-day window + @{ Msg = 'c6'; Age = 6 } # just INSIDE the 7-day window + )) { + $iso = $now.AddDays(-$c.Age).ToString('yyyy-MM-ddTHH:mm:ss') + Set-Content -Path (Join-Path $fixtureRepo "$($c.Msg).txt") -Value $c.Msg + git -C $fixtureRepo add -A 2>&1 | Out-Null + $env:GIT_AUTHOR_DATE = $iso + $env:GIT_COMMITTER_DATE = $iso # --since filters on committer date + try { + git -C $fixtureRepo commit -q -m $c.Msg 2>&1 | Out-Null + } finally { + Remove-Item Env:GIT_AUTHOR_DATE -ErrorAction SilentlyContinue + Remove-Item Env:GIT_COMMITTER_DATE -ErrorAction SilentlyContinue + } + } + # Get-RecentCommitCount resolves `origin/`, so publish a remote-tracking + # ref. Targeting HEAD keeps this branch-name agnostic (works whether git + # defaults the initial branch to 'main' or 'master'). + git -C $fixtureRepo update-ref refs/remotes/origin/main HEAD 2>&1 | Out-Null + + # Point the dot-sourced detector helper at the fixture for these assertions, + # then restore $Repo in `finally` so later tests are untouched. + $Repo = $fixtureRepo + Assert-Eq -Label "recency window: 7d counts only the 6-day-old commit" -Expected 1 -Actual (Get-RecentCommitCount -Ref 'main' -Days 7) + Assert-Eq -Label "recency window: 10d also includes the 8-day-old commit" -Expected 2 -Actual (Get-RecentCommitCount -Ref 'main' -Days 10) + Assert-Eq -Label "recency window: 60d includes all three commits" -Expected 3 -Actual (Get-RecentCommitCount -Ref 'main' -Days 60) + Assert-Eq -Label "recency window: 1d window -> 0 (the idle / no-activity case)" -Expected 0 -Actual (Get-RecentCommitCount -Ref 'main' -Days 1) + # The `origin/`-prefixed ref form must resolve identically (no double prefix). + Assert-Eq -Label "recency window: explicit origin/ ref resolves the same" -Expected 1 -Actual (Get-RecentCommitCount -Ref 'origin/main' -Days 7) + } finally { + $Repo = $savedRepo + if (Test-Path $fixtureRepo) { Remove-Item -Recurse -Force $fixtureRepo } + } } # ─────────── E2E: Run detection against this repo and validate trackers ─────────── @@ -592,7 +648,15 @@ if (-not $SkipE2E) { Assert-Eq -Label "SR8 branchName" -Expected 'release/10.0.1xx-sr8' -Actual $sr8.branchName Assert-Eq -Label "SR8 branchExists = true" -Expected $true -Actual $sr8.branchExists Assert-Eq -Label "SR8 expectedTag = 10.0.80" -Expected '10.0.80' -Actual $sr8.expectedTag - Assert-Eq -Label "SR8 hasRecentActivity = true" -Expected $true -Actual $sr8.hasRecentActivity + # hasRecentActivity is a 7-day-window signal (git log --since=7.days + # against the live branch), so its VALUE is wall-clock dependent and + # MUST NOT be pinned here — SR8 idling >7 days at the tail of a cycle + # is a NORMAL state that would (correctly) report $false. Assert only + # that the detector emits it as a real [bool]. The window math itself + # is covered deterministically by the synthetic-fixture unit test + # ([Unit] Get-RecentCommitCount recency window). + Assert-Eq -Label "SR8 hasRecentActivity is a [bool] (value is date-dependent)" ` + -Expected $true -Actual ($sr8.hasRecentActivity -is [bool]) Assert-Eq -Label "SR8 regression labels" ` -Expected 'regressed-in-10.0.70,regressed-in-10.0.80' ` -Actual ($sr8.regressionLabels -join ',') @@ -613,7 +677,9 @@ if (-not $SkipE2E) { Assert-Eq -Label "SR9 priorSrBranch = SR8 branch" ` -Expected 'release/10.0.1xx-sr8' -Actual $sr9.priorSrBranch Assert-Eq -Label "SR9 expectedPatch = 90" -Expected 90 -Actual $sr9.expectedPatch - Assert-Eq -Label "SR9 hasRecentActivity = true" -Expected $true -Actual $sr9.hasRecentActivity + # Same 7-day-window caveat as SR8: don't pin the value, assert the type. + Assert-Eq -Label "SR9 hasRecentActivity is a [bool] (value is date-dependent)" ` + -Expected $true -Actual ($sr9.hasRecentActivity -is [bool]) Assert-Eq -Label "SR9 regression labels" ` -Expected 'regressed-in-10.0.80,regressed-in-10.0.90' ` -Actual ($sr9.regressionLabels -join ',') @@ -621,12 +687,17 @@ if (-not $SkipE2E) { Write-Host " ❌ SR9 tracker missing" -ForegroundColor Red; $script:failed++ } - # Active SRs (the ones the workflow will actually post) all have activity. - # SR7 shipped 2026-06-05 (no longer in the tracker set); only SR8 + SR9 are active. + # Every active SR tracker must EXPOSE a hasRecentActivity flag, but that + # flag is a 7-day-window signal (git log --since=7.days), NOT a synonym + # for "active": an active SR can legitimately sit idle for >7 days near + # the tail of a cycle and report hasRecentActivity=$false. So assert the + # flag is a real [bool] — never a hardcoded, date-dependent $true. SR7 + # shipped 2026-06-05 and is no longer in the tracker set; only SR8 + SR9 + # are active. foreach ($srNum in @(8, 9)) { if ($bySr.ContainsKey($srNum)) { - Assert-Eq -Label "SR$srNum hasRecentActivity == true (active SR)" ` - -Expected $true -Actual $bySr[$srNum].hasRecentActivity + Assert-Eq -Label "SR$srNum hasRecentActivity is a [bool] (active SR; value date-dependent)" ` + -Expected $true -Actual ($bySr[$srNum].hasRecentActivity -is [bool]) } } } @@ -706,8 +777,11 @@ if (-not $SkipE2E) { -Expected 'release/11.0.1xx-preview6' -Actual $preview6.branchName Assert-Eq -Label "preview6 branchExists = false (no branch yet)" ` -Expected $false -Actual $preview6.branchExists - Assert-Eq -Label "preview6 hasRecentActivity = true (active preview cycle)" ` - -Expected $true -Actual $preview6.hasRecentActivity + # hasRecentActivity is a 7-day-window signal, not a marker of an + # "active preview cycle" — net11.0 can idle >7 days and report $false. + # Assert the flag's TYPE, not its date-dependent value. + Assert-Eq -Label "preview6 hasRecentActivity is a [bool] (value date-dependent)" ` + -Expected $true -Actual ($preview6.hasRecentActivity -is [bool]) Assert-Eq -Label "preview6 regressionLabels carries previewN-1 + previewN" ` -Expected 'regressed-in-11.0.0-preview5,regressed-in-11.0.0-preview6' ` -Actual ($preview6.regressionLabels -join ',') From 73abae830594f7fe025c32ccebccd805c90621db Mon Sep 17 00:00:00 2001 From: Shane Neuville <5375137+PureWeen@users.noreply.github.com> Date: Thu, 18 Jun 2026 13:13:47 -0500 Subject: [PATCH 2/3] Harden recency fixture: hermetic gpgsign, setup guard, safe cleanup Adversarial review round-1 hardening of the synthetic recency fixture: - Force commit.gpgsign=false locally so the throwaway repo builds on dev machines that enable commit signing globally (no key -> 0 commits). - Add a precondition guard asserting 3 commits on origin/main, so a swallowed git misconfig fails loudly instead of as a cryptic 'unknown revision' later. - Add -ErrorAction SilentlyContinue to the finally-block cleanup so a cleanup hiccup can't mask a real failure from the try body. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../tests/Test-ReleaseReadiness.ps1 | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 index 3b48645d18ea..f7b574d14c13 100644 --- a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 @@ -548,6 +548,10 @@ if (-not (Test-Path $detectScriptPath)) { git -C $fixtureRepo init -q 2>&1 | Out-Null git -C $fixtureRepo config user.email 'rr-test@example.com' 2>&1 | Out-Null git -C $fixtureRepo config user.name 'RR Test' 2>&1 | Out-Null + # Keep the fixture hermetic: a developer with `commit.gpgsign=true` set + # globally (but no signing key for this throwaway repo) would otherwise hit + # "gpg failed to sign the data" and produce zero commits. Force it off locally. + git -C $fixtureRepo config commit.gpgsign false 2>&1 | Out-Null # Three commits at known ages relative to "now". The 1-day margins on either # side of the 7-day window keep every assertion robust (no boundary fuzz). @@ -574,6 +578,15 @@ if (-not (Test-Path $detectScriptPath)) { # defaults the initial branch to 'main' or 'master'). git -C $fixtureRepo update-ref refs/remotes/origin/main HEAD 2>&1 | Out-Null + # Fail LOUDLY (and early) if the fixture didn't end up with the 3 commits the + # assertions below depend on — e.g. a machine-level git misconfig swallowed + # by `2>&1 | Out-Null`. Without this guard a broken setup surfaces only as a + # cryptic "unknown revision origin/main" from Get-RecentCommitCount later. + $fixtureCommitCount = (& git -C $fixtureRepo rev-list --count origin/main 2>$null) + if ($LASTEXITCODE -ne 0 -or "$fixtureCommitCount".Trim() -ne '3') { + throw "Recency fixture setup failed: expected 3 commits on 'origin/main', got '$fixtureCommitCount' (git exit $LASTEXITCODE). Check this machine's git config (e.g. commit.gpgsign / hooks)." + } + # Point the dot-sourced detector helper at the fixture for these assertions, # then restore $Repo in `finally` so later tests are untouched. $Repo = $fixtureRepo @@ -585,7 +598,9 @@ if (-not (Test-Path $detectScriptPath)) { Assert-Eq -Label "recency window: explicit origin/ ref resolves the same" -Expected 1 -Actual (Get-RecentCommitCount -Ref 'origin/main' -Days 7) } finally { $Repo = $savedRepo - if (Test-Path $fixtureRepo) { Remove-Item -Recurse -Force $fixtureRepo } + # SilentlyContinue so a cleanup hiccup (e.g. a transient file lock on .git) + # can't throw from `finally` and mask a real failure from the `try` body. + if (Test-Path $fixtureRepo) { Remove-Item -Recurse -Force $fixtureRepo -ErrorAction SilentlyContinue } } } From ee518f68a197f2bed3b9f59a2c934c0287aeaaa9 Mon Sep 17 00:00:00 2001 From: Shane Neuville <5375137+PureWeen@users.noreply.github.com> Date: Thu, 18 Jun 2026 13:27:27 -0500 Subject: [PATCH 3/3] Round-2 review: pin count->flag mapping, neutralize hooks, restore env Adversarial review round-2 consensus fixes: - Mapping coverage (2/3): the live assertions only checked hasRecentActivity is a [bool]; the detector's actual wiring (hasRecentActivity = recentCommitCount > 0) was unasserted, so an inverted/hardcoded mapping could ship silently. Add a date-INDEPENDENT invariant on the SR and preview trackers: hasRecentActivity -eq (recentCommitCount > 0). Both fields come off the same tracker at the same instant, so it never flakes yet still trips on a broken mapping. (+3 assertions: SR8, SR9, preview6.) - Hermetic hooks (2/3): a global core.hooksPath or an init.templateDir that seeds .git/hooks could install a pre-commit hook that rejects the synthetic commits. Redirect hook lookup to an empty path under .git (overrides global hooksPath and bypasses templated hooks). - Env restore (1/3): save/restore ambient GIT_AUTHOR_DATE/GIT_COMMITTER_DATE around the commit loop instead of blindly Remove-Item-ing them, so a caller that pre-set those vars isn't left mutated. Full suite: 555/0 (E2E), 489/0 (-SkipE2E). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../tests/Test-ReleaseReadiness.ps1 | 62 +++++++++++++------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 index f7b574d14c13..90ee0e1a6dd5 100644 --- a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 @@ -548,30 +548,45 @@ if (-not (Test-Path $detectScriptPath)) { git -C $fixtureRepo init -q 2>&1 | Out-Null git -C $fixtureRepo config user.email 'rr-test@example.com' 2>&1 | Out-Null git -C $fixtureRepo config user.name 'RR Test' 2>&1 | Out-Null - # Keep the fixture hermetic: a developer with `commit.gpgsign=true` set - # globally (but no signing key for this throwaway repo) would otherwise hit - # "gpg failed to sign the data" and produce zero commits. Force it off locally. - git -C $fixtureRepo config commit.gpgsign false 2>&1 | Out-Null + # Keep the fixture hermetic against the host's git config — otherwise a + # developer/CI machine could break the synthetic commits in ways unrelated + # to the code under test: + # - commit.gpgsign=true with no key for this throwaway repo -> "gpg failed + # to sign the data" -> zero commits. + # - a global core.hooksPath, or an init.templateDir that seeds .git/hooks, + # installing a pre-commit/commit-msg hook (linters, ticket-number + # enforcement, etc.) -> commits rejected. + # Force signing off and redirect hook lookup to an empty (nonexistent) path + # under .git so neither can interfere. A local core.hooksPath overrides any + # global one AND bypasses templated .git/hooks. The setup guard below still + # fails loud if anything else goes wrong. + git -C $fixtureRepo config commit.gpgsign false 2>&1 | Out-Null + git -C $fixtureRepo config core.hooksPath (Join-Path (Join-Path $fixtureRepo '.git') '_disabled-hooks') 2>&1 | Out-Null # Three commits at known ages relative to "now". The 1-day margins on either # side of the 7-day window keep every assertion robust (no boundary fuzz). $now = Get-Date - foreach ($c in @( - @{ Msg = 'c30'; Age = 30 } # well outside any window under test - @{ Msg = 'c8'; Age = 8 } # just OUTSIDE the 7-day window - @{ Msg = 'c6'; Age = 6 } # just INSIDE the 7-day window - )) { - $iso = $now.AddDays(-$c.Age).ToString('yyyy-MM-ddTHH:mm:ss') - Set-Content -Path (Join-Path $fixtureRepo "$($c.Msg).txt") -Value $c.Msg - git -C $fixtureRepo add -A 2>&1 | Out-Null - $env:GIT_AUTHOR_DATE = $iso - $env:GIT_COMMITTER_DATE = $iso # --since filters on committer date - try { + # Preserve any ambient GIT_*_DATE the caller set: we override them per commit + # to control dates, then restore the originals so a later test in this process + # (or the parent environment) is never left mutated. + $priorAuthorDate = $env:GIT_AUTHOR_DATE + $priorCommitterDate = $env:GIT_COMMITTER_DATE + try { + foreach ($c in @( + @{ Msg = 'c30'; Age = 30 } # well outside any window under test + @{ Msg = 'c8'; Age = 8 } # just OUTSIDE the 7-day window + @{ Msg = 'c6'; Age = 6 } # just INSIDE the 7-day window + )) { + $iso = $now.AddDays(-$c.Age).ToString('yyyy-MM-ddTHH:mm:ss') + Set-Content -Path (Join-Path $fixtureRepo "$($c.Msg).txt") -Value $c.Msg + git -C $fixtureRepo add -A 2>&1 | Out-Null + $env:GIT_AUTHOR_DATE = $iso + $env:GIT_COMMITTER_DATE = $iso # --since filters on committer date git -C $fixtureRepo commit -q -m $c.Msg 2>&1 | Out-Null - } finally { - Remove-Item Env:GIT_AUTHOR_DATE -ErrorAction SilentlyContinue - Remove-Item Env:GIT_COMMITTER_DATE -ErrorAction SilentlyContinue } + } finally { + if ($null -eq $priorAuthorDate) { Remove-Item Env:GIT_AUTHOR_DATE -ErrorAction SilentlyContinue } else { $env:GIT_AUTHOR_DATE = $priorAuthorDate } + if ($null -eq $priorCommitterDate) { Remove-Item Env:GIT_COMMITTER_DATE -ErrorAction SilentlyContinue } else { $env:GIT_COMMITTER_DATE = $priorCommitterDate } } # Get-RecentCommitCount resolves `origin/`, so publish a remote-tracking # ref. Targeting HEAD keeps this branch-name agnostic (works whether git @@ -713,6 +728,12 @@ if (-not $SkipE2E) { if ($bySr.ContainsKey($srNum)) { Assert-Eq -Label "SR$srNum hasRecentActivity is a [bool] (active SR; value date-dependent)" ` -Expected $true -Actual ($bySr[$srNum].hasRecentActivity -is [bool]) + # Pin the detector's count->flag WIRING (hasRecentActivity = recentCommitCount > 0) + # without pinning the date-dependent value: both fields come off the SAME tracker + # computed at the SAME instant, so this invariant holds no matter how active the + # branch is, yet still catches an inverted/hardcoded mapping. + Assert-Eq -Label "SR$srNum hasRecentActivity == (recentCommitCount > 0) [mapping invariant]" ` + -Expected $true -Actual ($bySr[$srNum].hasRecentActivity -eq ([int]$bySr[$srNum].recentCommitCount -gt 0)) } } } @@ -797,6 +818,11 @@ if (-not $SkipE2E) { # Assert the flag's TYPE, not its date-dependent value. Assert-Eq -Label "preview6 hasRecentActivity is a [bool] (value date-dependent)" ` -Expected $true -Actual ($preview6.hasRecentActivity -is [bool]) + # Pin the count->flag WIRING (hasRecentActivity = recentCommitCount > 0) for the + # preview construction path too — same-instant fields, so date-independent yet it + # still trips on an inverted/hardcoded mapping. + Assert-Eq -Label "preview6 hasRecentActivity == (recentCommitCount > 0) [mapping invariant]" ` + -Expected $true -Actual ($preview6.hasRecentActivity -eq ([int]$preview6.recentCommitCount -gt 0)) Assert-Eq -Label "preview6 regressionLabels carries previewN-1 + previewN" ` -Expected 'regressed-in-11.0.0-preview5,regressed-in-11.0.0-preview6' ` -Actual ($preview6.regressionLabels -join ',')