Skip to content

Commit a472895

Browse files
ReubenBondReubenBond
committed
fix(ci): make coverage identities deterministic
1 parent 739a2d4 commit a472895

4 files changed

Lines changed: 110 additions & 16 deletions

File tree

‎.github/scripts/CoverageSummary.cs‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ private static int ReadCoverageReport(
156156
var methodIdentity = string.Empty;
157157
var linesBelongToMethod = false;
158158
var methodBranchLines = new HashSet<int>();
159+
var pendingClassBranches = new List<PendingBranchLine>();
159160
RepositorySourcePath? sourcePath = null;
160161
try
161162
{
@@ -172,6 +173,7 @@ private static int ReadCoverageReport(
172173
methodIdentity = string.Empty;
173174
linesBelongToMethod = false;
174175
methodBranchLines.Clear();
176+
pendingClassBranches.Clear();
175177
sourcePath = GetRepositoryPath(reader.GetAttribute("filename"), normalizedSourceRoot);
176178
if (reader.IsEmptyElement)
177179
{
@@ -235,7 +237,10 @@ private static int ReadCoverageReport(
235237
}
236238
else if (!methodBranchLines.Contains(lineNumber))
237239
{
238-
branchSiteIdentity = $"{classIdentity}\0<non-method>";
240+
pendingClassBranches.Add(
241+
new PendingBranchLine(
242+
lineNumber,
243+
reader.GetAttribute("condition-coverage") ?? string.Empty));
239244
}
240245
}
241246

@@ -266,13 +271,32 @@ private static int ReadCoverageReport(
266271
}
267272
else if (reader.Depth == classDepth && reader.LocalName == "class")
268273
{
274+
if (sourcePath is not null)
275+
{
276+
foreach (var pendingBranch in pendingClassBranches)
277+
{
278+
if (!methodBranchLines.Contains(pendingBranch.LineNumber))
279+
{
280+
ProcessBranch(
281+
pendingBranch.ConditionCoverage,
282+
pendingBranch.LineNumber,
283+
$"{classIdentity}\0<non-method>",
284+
reportPath,
285+
sourcePath,
286+
measuredBranches,
287+
measuredBranchTotals);
288+
}
289+
}
290+
}
291+
269292
classDepth = -1;
270293
methodDepth = -1;
271294
linesDepth = -1;
272295
classIdentity = string.Empty;
273296
methodIdentity = string.Empty;
274297
linesBelongToMethod = false;
275298
methodBranchLines.Clear();
299+
pendingClassBranches.Clear();
276300
sourcePath = null;
277301
}
278302
}
@@ -327,7 +351,25 @@ private static void ProcessLine(
327351
return;
328352
}
329353

330-
var conditionCoverage = reader.GetAttribute("condition-coverage") ?? string.Empty;
354+
ProcessBranch(
355+
reader.GetAttribute("condition-coverage") ?? string.Empty,
356+
lineNumber,
357+
branchSiteIdentity,
358+
reportPath,
359+
sourcePath,
360+
measuredBranches,
361+
measuredBranchTotals);
362+
}
363+
364+
private static void ProcessBranch(
365+
string conditionCoverage,
366+
int lineNumber,
367+
string branchSiteIdentity,
368+
string reportPath,
369+
RepositorySourcePath sourcePath,
370+
Dictionary<string, Dictionary<string, bool>> measuredBranches,
371+
Dictionary<string, Dictionary<string, int>> measuredBranchTotals)
372+
{
331373
var match = ConditionCoverage.Match(conditionCoverage);
332374
if (!match.Success)
333375
{
@@ -468,5 +510,7 @@ private static void AssertNotReparsePoint(string path)
468510
}
469511
}
470512

513+
private sealed record PendingBranchLine(int LineNumber, string ConditionCoverage);
514+
471515
private sealed record RepositorySourcePath(string RelativePath, string RepositoryPath);
472516
}

‎.github/scripts/get-coverage-input-fingerprint.ps1‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,10 @@ if ($inputs.Count -eq 0) {
5151
throw 'The coverage input manifest is empty'
5252
}
5353

54+
$sortedInputs = @($inputs)
55+
[Array]::Sort($sortedInputs, [StringComparer]::Ordinal)
5456
$entries = @(
55-
foreach ($relativePath in @($inputs | Sort-Object)) {
57+
foreach ($relativePath in $sortedInputs) {
5658
$currentPath = $resolvedRepositoryRoot
5759
Assert-NotReparsePoint $currentPath
5860
foreach ($part in $relativePath.Split('/')) {

‎.github/scripts/test-summarize-coverage.ps1‎

Lines changed: 58 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,26 @@ try {
443443
Assert-Equal 6 $summary.total_branches 'Distinct branch site total branches differ.'
444444
}
445445

446+
Invoke-Test 'does not double count class lines which precede methods' {
447+
$testCase = New-TestCase
448+
$sourceFile = [Security.SecurityElement]::Escape((Join-Path $testCase.SourceRoot 'Example.cs'))
449+
$xml = @"
450+
<coverage><packages><package><classes>
451+
<class name="ContainingType" filename="$sourceFile">
452+
<lines><line number="10" hits="1" branch="True" condition-coverage="50% (1/2)" /></lines>
453+
<methods><method name="Method" signature="()"><lines>
454+
<line number="10" hits="1" branch="True" condition-coverage="50% (1/2)" />
455+
</lines></method></methods>
456+
</class>
457+
</classes></package></packages></coverage>
458+
"@
459+
[void] (Write-Report $testCase $xml)
460+
Invoke-Summarizer $testCase
461+
$summary = Get-Content -Raw $testCase.JsonOutput | ConvertFrom-Json
462+
Assert-Equal 1 $summary.covered_branches 'Reordered covered branches differ.'
463+
Assert-Equal 2 $summary.total_branches 'Reordered total branches differ.'
464+
}
465+
446466
Invoke-Test 'rejects inconsistent branch coverage' {
447467
$testCase = New-TestCase
448468
$lines = @'
@@ -820,30 +840,41 @@ exit 0
820840
$expectedArtifacts = Join-Path $testCase.Root 'expected.txt'
821841
[IO.File]::WriteAllLines(
822842
$expectedArtifacts,
823-
@('test_output_first', 'test_output_second'),
843+
@('test_output_a', 'test_output_B'),
824844
[Text.UTF8Encoding]::new($false)
825845
)
826-
$firstArtifact = Join-Path $testCase.ReportDirectory 'coverage_test_output_first'
846+
$firstArtifact = Join-Path $testCase.ReportDirectory 'coverage_test_output_a'
827847
[void] (New-Item -ItemType Directory -Path $firstArtifact)
828848
[IO.File]::WriteAllText(
829-
(Join-Path $firstArtifact 'test_output_first.cobertura.xml'),
849+
(Join-Path $firstArtifact 'test_output_a.cobertura.xml'),
830850
'<coverage />',
831851
[Text.UTF8Encoding]::new($false)
832852
)
833-
Write-ArtifactMetadata $firstArtifact 'test_output_first'
853+
Write-ArtifactMetadata $firstArtifact 'test_output_a'
834854
Assert-Throws `
835855
{ Invoke-ArtifactValidator $testCase.ReportDirectory $expectedArtifacts } `
836-
'Missing: coverage_test_output_second'
856+
'Missing: coverage_test_output_B'
837857

838-
$secondArtifact = Join-Path $testCase.ReportDirectory 'coverage_test_output_second'
858+
$secondArtifact = Join-Path $testCase.ReportDirectory 'coverage_test_output_B'
839859
[void] (New-Item -ItemType Directory -Path $secondArtifact)
840860
[IO.File]::WriteAllText(
841-
(Join-Path $secondArtifact 'test_output_second.cobertura.xml'),
861+
(Join-Path $secondArtifact 'test_output_B.cobertura.xml'),
842862
'<coverage />',
843863
[Text.UTF8Encoding]::new($false)
844864
)
845-
Write-ArtifactMetadata $secondArtifact 'test_output_second'
865+
Write-ArtifactMetadata $secondArtifact 'test_output_B'
846866
Invoke-ArtifactValidator $testCase.ReportDirectory $expectedArtifacts | Out-Null
867+
$validationPath = Join-Path $testCase.Root 'validation.json'
868+
$firstManifestSha = (Get-Content -Raw $validationPath | ConvertFrom-Json).manifest_sha256
869+
$previousCulture = [Threading.Thread]::CurrentThread.CurrentCulture
870+
try {
871+
[Threading.Thread]::CurrentThread.CurrentCulture = [Globalization.CultureInfo]::GetCultureInfo('tr-TR')
872+
Invoke-ArtifactValidator $testCase.ReportDirectory $expectedArtifacts | Out-Null
873+
} finally {
874+
[Threading.Thread]::CurrentThread.CurrentCulture = $previousCulture
875+
}
876+
$secondManifestSha = (Get-Content -Raw $validationPath | ConvertFrom-Json).manifest_sha256
877+
Assert-Equal $firstManifestSha $secondManifestSha 'Artifact manifest fingerprints must use ordinal ordering.'
847878

848879
$unexpectedArtifact = Join-Path $testCase.ReportDirectory 'coverage_test_output_unexpected'
849880
[void] (New-Item -ItemType Directory -Path $unexpectedArtifact)
@@ -979,22 +1010,37 @@ exit 0
9791010
$testCase = New-TestCase
9801011
$repository = Join-Path $testCase.Root 'repository'
9811012
[void] (New-Item -ItemType Directory -Path (Join-Path $repository '.github/scripts') -Force)
982-
[IO.File]::WriteAllText((Join-Path $repository '.github/scripts/first.ps1'), 'first', [Text.UTF8Encoding]::new($false))
983-
[IO.File]::WriteAllText((Join-Path $repository 'global.json'), '{}', [Text.UTF8Encoding]::new($false))
1013+
[IO.File]::WriteAllText((Join-Path $repository '.github/scripts/a.ps1'), 'first', [Text.UTF8Encoding]::new($false))
1014+
[IO.File]::WriteAllText((Join-Path $repository '.github/scripts/B.ps1'), 'second', [Text.UTF8Encoding]::new($false))
9841015
$manifest = Join-Path $testCase.Root 'inputs.txt'
9851016
[IO.File]::WriteAllLines(
9861017
$manifest,
987-
@('global.json', '.github/scripts/first.ps1'),
1018+
@('.github/scripts/a.ps1', '.github/scripts/B.ps1'),
9881019
[Text.UTF8Encoding]::new($false)
9891020
)
9901021
$firstOutput = Join-Path $testCase.Root 'first.json'
1022+
$reorderedOutput = Join-Path $testCase.Root 'reordered.json'
9911023
$secondOutput = Join-Path $testCase.Root 'second.json'
9921024
& $coverageInputFingerprintScriptPath -RepositoryRoot $repository -Manifest $manifest -JsonOutput $firstOutput
993-
[IO.File]::WriteAllText((Join-Path $repository '.github/scripts/first.ps1'), 'changed', [Text.UTF8Encoding]::new($false))
1025+
[IO.File]::WriteAllLines(
1026+
$manifest,
1027+
@('.github/scripts/B.ps1', '.github/scripts/a.ps1'),
1028+
[Text.UTF8Encoding]::new($false)
1029+
)
1030+
$previousCulture = [Threading.Thread]::CurrentThread.CurrentCulture
1031+
try {
1032+
[Threading.Thread]::CurrentThread.CurrentCulture = [Globalization.CultureInfo]::GetCultureInfo('tr-TR')
1033+
& $coverageInputFingerprintScriptPath -RepositoryRoot $repository -Manifest $manifest -JsonOutput $reorderedOutput
1034+
} finally {
1035+
[Threading.Thread]::CurrentThread.CurrentCulture = $previousCulture
1036+
}
1037+
[IO.File]::WriteAllText((Join-Path $repository '.github/scripts/a.ps1'), 'changed', [Text.UTF8Encoding]::new($false))
9941038
& $coverageInputFingerprintScriptPath -RepositoryRoot $repository -Manifest $manifest -JsonOutput $secondOutput
9951039
$first = Get-Content -Raw $firstOutput | ConvertFrom-Json
1040+
$reordered = Get-Content -Raw $reorderedOutput | ConvertFrom-Json
9961041
$second = Get-Content -Raw $secondOutput | ConvertFrom-Json
9971042
Assert-Equal 2 $first.files 'Coverage input file count differs.'
1043+
Assert-Equal $first.sha256 $reordered.sha256 'Coverage input fingerprints must use ordinal ordering.'
9981044
Assert-Equal $false ($first.sha256 -eq $second.sha256) 'Coverage input changes must change the fingerprint.'
9991045
}
10001046

‎.github/scripts/validate-coverage-artifacts.ps1‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ foreach ($line in [IO.File]::ReadAllLines($resolvedExpectedArtifacts)) {
4646
if ($expected.Count -eq 0) {
4747
throw 'The expected coverage artifact set is empty'
4848
}
49-
$manifestText = (@($expected.Keys | Sort-Object) -join "`n") + "`n"
49+
$sortedArtifactNames = @($expected.Keys)
50+
[Array]::Sort($sortedArtifactNames, [StringComparer]::Ordinal)
51+
$manifestText = ($sortedArtifactNames -join "`n") + "`n"
5052
$manifestBytes = [Text.UTF8Encoding]::new($false).GetBytes($manifestText)
5153
$manifestSha256 = [Convert]::ToHexString([Security.Cryptography.SHA256]::HashData($manifestBytes)).ToLowerInvariant()
5254

0 commit comments

Comments
 (0)