From 93b035c72fdd0cc622b7a83694d7a10158101d39 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Tue, 3 Nov 2026 11:17:44 +0000 Subject: [PATCH 1/3] refactor: hand ci-unit-tests reporting to the test runner and ReportGenerator Test results: the HTML logger beside the TRX, and the excluded tests as vstest lists them by category, uploaded together. Coverage: ReportGenerator merges the per-project reports, keeps only files in the checkout outside .ci, and produces the Html report; the headline is read from its JsonSummary. The summary is #45's table plus one coverage line and links. Removed: the solution comparison diagnostic, the coverage PR comment and its pr_number input. upload-artifact moves to v7.0.2. --- .github/actions/ci-unit-tests/action.yml | 138 ++++++++++------------- 1 file changed, 59 insertions(+), 79 deletions(-) diff --git a/.github/actions/ci-unit-tests/action.yml b/.github/actions/ci-unit-tests/action.yml index c83696d..9f07aa1 100644 --- a/.github/actions/ci-unit-tests/action.yml +++ b/.github/actions/ci-unit-tests/action.yml @@ -1,7 +1,7 @@ name: "CI Unit Tests" description: | - Discovers and runs unit tests for the caller repo, generating code coverage reports - and posting results to the PR. + Discovers and runs unit tests for the caller repo. Results and coverage are + published as artifacts by the test runner and ReportGenerator. inputs: app_id: @@ -16,12 +16,6 @@ inputs: description: Fallback token when App credentials are absent. required: false default: "" - pr_number: - description: > - PR number in the caller repo. Proxy workflows pass this so the - coverage comment targets the right PR in cross-org contexts. - required: false - default: "" test_toolkit_repo: description: > Test_Toolkit repository (owner/repo). Override to point at a fork. @@ -119,47 +113,6 @@ runs: Write-Host "::notice title=UnitTests::Discovered $($projects.Count) test project(s)" foreach ($p in $projects) { Write-Host " $(Resolve-Path -Relative $p)" } - # Diagnostic only, never fails the check. Reports where the new selection - # differs from what the previous .sln-driven implementation would have - # picked, so a repo whose set changes is visible rather than silent. - - name: Compare against the solution-driven selection - if: steps.test_check.outputs.found == 'true' - shell: pwsh - env: - PROJECT_LIST: ${{ runner.temp }}/ci-unit-tests-projects.txt - run: | - $new = @(Get-Content $env:PROJECT_LIST | Where-Object { $_ }) - - # Exactly the previous behaviour: non-recursive .sln lookup, first - # match, quoted paths ending in Tests.csproj. - $slns = Get-ChildItem -Path ".ci/unit-tests" -Filter "*.sln" -ErrorAction SilentlyContinue - $old = @() - if ($slns) { - $sln = $slns[0].FullName - $slnDir = Split-Path $sln -Parent - $old = @(Select-String -Path $sln -Pattern '"(?

[^"]+Tests\.csproj)"' | ForEach-Object { - $rel = $_.Matches[0].Groups['p'].Value -replace '\\','/' - $abs = Join-Path $slnDir $rel - if (Test-Path $abs) { (Resolve-Path $abs).Path } - }) - } - - $onlyNew = @($new | Where-Object { $old -notcontains $_ }) - $onlyOld = @($old | Where-Object { $new -notcontains $_ }) - - if (-not $slns) { - Write-Host "::notice title=UnitTests::No .sln directly under .ci/unit-tests, so the previous implementation selected nothing here and ran no tests." - } - if ($onlyNew) { - Write-Host "::notice title=UnitTests::Newly discovered, not selected by the solution: $(($onlyNew | ForEach-Object { Split-Path $_ -Leaf }) -join ', ')" - } - if ($onlyOld) { - Write-Host "::warning title=UnitTests::Listed in the solution but not detected as a test project: $(($onlyOld | ForEach-Object { Split-Path $_ -Leaf }) -join ', ')" - } - if ($slns -and -not $onlyNew -and -not $onlyOld) { - Write-Host "::notice title=UnitTests::Selection matches the solution-driven selection." - } - - name: Mint dependency token id: dep-token if: steps.test_check.outputs.found == 'true' @@ -329,10 +282,20 @@ runs: continue } + # The excluded tests never reach the results files, so vstest lists + # them by category, as written, beside the run's own output. + New-Item -ItemType Directory -Force -Path TestResults | Out-Null + foreach ($dll in [regex]::Matches($listOut, '(?m)^Test run for (.+?\.dll) \(')) { + dotnet vstest $dll.Groups[1].Value /ListFullyQualifiedTests ` + "/ListTestsTargetPath:$(Join-Path $PWD "TestResults/$name.excluded.txt")" ` + "/TestCaseFilter:TestCategory=$category" | Out-Null + } + Write-Host "::group::dotnet test $name --filter $filter" dotnet test $proj -c Test --no-build --filter $filter ` --collect:"XPlat Code Coverage" ` - --logger "trx;LogFileName=$name.trx" --results-directory TestResults + --logger "trx;LogFileName=$name.trx" --logger "html;LogFileName=$name.html" ` + --results-directory TestResults $exit = $LASTEXITCODE Write-Host "::endgroup::" @@ -470,41 +433,58 @@ runs: if ($testFailed) { Write-Host "::error title=UnitTests::Test runs failed for project(s): $($testFailed -join ', ')" } if ($buildFailed -or $undetermined -or $noTests -or $noneExecuted -or $noOutput -or $badResults -or $testFailed) { exit 1 } - - name: Upload test results (TRX) - if: always() && hashFiles('TestResults/**/*.trx') != '' - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + - name: Upload test results + id: test_results + if: always() && hashFiles('TestResults/*.trx') != '' + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: name: test-results-${{ github.run_id }} - path: TestResults/ + path: | + TestResults/*.trx + TestResults/*.html + TestResults/*.excluded.txt retention-days: 7 - - name: Generate code coverage report - id: generate_coverage - if: always() && hashFiles('TestResults/**/coverage.cobertura.xml') != '' + # ReportGenerator merges the per-project reports and keeps only files in + # this checkout outside .ci. Dependencies are cloned outside the checkout, + # so their source never reaches the report. Reporting only: nothing here + # can change the check's outcome. + - name: Coverage report + id: coverage + if: always() && hashFiles('TestResults/*/coverage.cobertura.xml') != '' continue-on-error: true shell: pwsh run: | - New-Item -ItemType Directory -Force -Path ".config" | Out-Null - Set-Content -Path ".config/dotnet-tools.json" -Value '{ - "version": 1, - "isRoot": true, - "tools": { - "dotnet-reportgenerator-globaltool": { - "version": "5.4.4", - "commands": ["reportgenerator"] - } - } - }' - dotnet tool restore - dotnet tool run reportgenerator -- "-reports:TestResults/*/coverage.cobertura.xml" "-targetdir:CodeCoverage" "-reporttypes:MarkdownSummaryGithub;Cobertura" - Get-Content CodeCoverage/SummaryGithub.md | Add-Content -Path $env:GITHUB_STEP_SUMMARY - - - name: Add Coverage PR Comment - if: always() && hashFiles('CodeCoverage/SummaryGithub.md') != '' && (github.event_name == 'pull_request' || inputs.pr_number) + $tools = Join-Path $env:RUNNER_TEMP 'reportgenerator' + dotnet tool install dotnet-reportgenerator-globaltool --version 5.4.4 --tool-path $tools + & (Join-Path $tools 'reportgenerator') "-reports:TestResults/*/coverage.cobertura.xml" ` + "-targetdir:CodeCoverage" "-reporttypes:Html;JsonSummary" ` + "-filefilters:+$env:GITHUB_WORKSPACE\*;-$env:GITHUB_WORKSPACE\.ci\*" + Move-Item CodeCoverage/Summary.json (Join-Path $env:RUNNER_TEMP 'coverage-summary.json') -Force + Remove-Item CodeCoverage/index.htm -ErrorAction SilentlyContinue + + - name: Upload the coverage report + id: coverage_report + if: always() && steps.coverage.outcome == 'success' continue-on-error: true - uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3.0.4 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: - recreate: false - path: CodeCoverage/SummaryGithub.md - number: ${{ inputs.pr_number || github.event.pull_request.number }} - GITHUB_TOKEN: ${{ steps.dep-token.outputs.token }} + name: coverage-report-${{ github.run_id }} + path: CodeCoverage/ + retention-days: 14 + + - name: Coverage and report links + if: always() && steps.test_check.outputs.found == 'true' + continue-on-error: true + shell: pwsh + env: + COVERAGE_URL: ${{ steps.coverage_report.outputs.artifact-url }} + RESULTS_URL: ${{ steps.test_results.outputs.artifact-url }} + run: | + $json = Join-Path $env:RUNNER_TEMP 'coverage-summary.json' + if (Test-Path $json) { + $s = (Get-Content $json -Raw | ConvertFrom-Json).summary + $branch = if ($null -ne $s.branchcoverage) { "$($s.branchcoverage)%" } else { 'n/a' } + "", "Coverage: line $($s.linecoverage)%, branch $branch" | Add-Content -Path $env:GITHUB_STEP_SUMMARY + } + "", "Reports: [coverage]($env:COVERAGE_URL), [test results]($env:RESULTS_URL)" | Add-Content -Path $env:GITHUB_STEP_SUMMARY From 5fb036b6bce5a05800e307d591ae1bc02d65ee31 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Tue, 3 Nov 2026 12:16:51 +0000 Subject: [PATCH 2/3] fix: collapse not-run names, say when coverage is not available --- .github/actions/ci-unit-tests/action.yml | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/.github/actions/ci-unit-tests/action.yml b/.github/actions/ci-unit-tests/action.yml index 9f07aa1..c083ba9 100644 --- a/.github/actions/ci-unit-tests/action.yml +++ b/.github/actions/ci-unit-tests/action.yml @@ -393,13 +393,9 @@ runs: } if ($partialNotRun) { - "" | Add-Content -Path $env:GITHUB_STEP_SUMMARY - "> **Not run.** These tests were admitted by the filter and the runner did not execute them, usually `Assert.Inconclusive` or `[Ignore]`. The check passed on the tests that did run." | + $names = @(foreach ($p in $partialNotRun) { foreach ($t in $p.Names) { "- ``$($p.Name)``: $t" } }) + @("", "

Not run ($($names.Count))", "") + $names + @("", "
") | Add-Content -Path $env:GITHUB_STEP_SUMMARY - "" | Add-Content -Path $env:GITHUB_STEP_SUMMARY - foreach ($p in $partialNotRun) { - foreach ($t in $p.Names) { "- ``$($p.Name)``: $t" | Add-Content -Path $env:GITHUB_STEP_SUMMARY } - } } foreach ($n in $allExcluded) { @@ -482,9 +478,11 @@ runs: RESULTS_URL: ${{ steps.test_results.outputs.artifact-url }} run: | $json = Join-Path $env:RUNNER_TEMP 'coverage-summary.json' - if (Test-Path $json) { - $s = (Get-Content $json -Raw | ConvertFrom-Json).summary - $branch = if ($null -ne $s.branchcoverage) { "$($s.branchcoverage)%" } else { 'n/a' } - "", "Coverage: line $($s.linecoverage)%, branch $branch" | Add-Content -Path $env:GITHUB_STEP_SUMMARY + $s = if (Test-Path $json) { (Get-Content $json -Raw | ConvertFrom-Json).summary } + $coverage = 'not available' + if ($s.coverablelines -gt 0) { + $branch = if ($null -ne $s.branchcoverage) { "$($s.branchcoverage)%" } else { 'n/a' } + $coverage = "line $($s.linecoverage)%, branch $branch" } + "", "Coverage: $coverage" | Add-Content -Path $env:GITHUB_STEP_SUMMARY "", "Reports: [coverage]($env:COVERAGE_URL), [test results]($env:RESULTS_URL)" | Add-Content -Path $env:GITHUB_STEP_SUMMARY From e2467d6635ed16375aab9a9236ec87ff6e553605 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Tue, 3 Nov 2026 12:41:46 +0000 Subject: [PATCH 3/3] fix: write only the report links that exist, tell no coverage data from nothing to measure --- .github/actions/ci-unit-tests/action.yml | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/.github/actions/ci-unit-tests/action.yml b/.github/actions/ci-unit-tests/action.yml index c083ba9..ea2edda 100644 --- a/.github/actions/ci-unit-tests/action.yml +++ b/.github/actions/ci-unit-tests/action.yml @@ -429,9 +429,13 @@ runs: if ($testFailed) { Write-Host "::error title=UnitTests::Test runs failed for project(s): $($testFailed -join ', ')" } if ($buildFailed -or $undetermined -or $noTests -or $noneExecuted -or $noOutput -or $badResults -or $testFailed) { exit 1 } + # Every step from here to the end is reporting only. Each is + # continue-on-error, so none can change the check's outcome, which the + # test run above has already decided. - name: Upload test results id: test_results if: always() && hashFiles('TestResults/*.trx') != '' + continue-on-error: true uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: name: test-results-${{ github.run_id }} @@ -443,8 +447,7 @@ runs: # ReportGenerator merges the per-project reports and keeps only files in # this checkout outside .ci. Dependencies are cloned outside the checkout, - # so their source never reaches the report. Reporting only: nothing here - # can change the check's outcome. + # so their source never reaches the report. - name: Coverage report id: coverage if: always() && hashFiles('TestResults/*/coverage.cobertura.xml') != '' @@ -479,10 +482,11 @@ runs: run: | $json = Join-Path $env:RUNNER_TEMP 'coverage-summary.json' $s = if (Test-Path $json) { (Get-Content $json -Raw | ConvertFrom-Json).summary } - $coverage = 'not available' - if ($s.coverablelines -gt 0) { - $branch = if ($null -ne $s.branchcoverage) { "$($s.branchcoverage)%" } else { 'n/a' } - $coverage = "line $($s.linecoverage)%, branch $branch" - } + $coverage = if (-not $s) { 'no data collected' } + elseif ($s.coverablelines -eq 0) { 'no coverable lines' } + else { "line $($s.linecoverage)%, branch $(if ($null -ne $s.branchcoverage) { "$($s.branchcoverage)%" } else { 'n/a' })" } "", "Coverage: $coverage" | Add-Content -Path $env:GITHUB_STEP_SUMMARY - "", "Reports: [coverage]($env:COVERAGE_URL), [test results]($env:RESULTS_URL)" | Add-Content -Path $env:GITHUB_STEP_SUMMARY + $links = @() + if ($env:COVERAGE_URL) { $links += "[coverage]($env:COVERAGE_URL)" } + if ($env:RESULTS_URL) { $links += "[test results]($env:RESULTS_URL)" } + if ($links) { "", "Reports: $($links -join ', ')" | Add-Content -Path $env:GITHUB_STEP_SUMMARY }