From 77a3641ef99da85805c6a5dab27a5ae9e55ce250 Mon Sep 17 00:00:00 2001 From: monsieurleberre Date: Fri, 12 Jun 2026 11:25:32 +0200 Subject: [PATCH 1/4] fix(sort-coverage-table): detect irongut tables without leading pipes (#18) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit irongut/CodeCoverageSummary v1.3.0 format: markdown emits tables without leading pipes, so the line.startswith('|') detection never matched and the sort was a byte-for-byte no-op in production. Detect a table as a header line (>=2 cells when split on '|') immediately followed by a separator line of '-', ':' and whitespace cells — with or without leading pipes — and extract the first cell from either shape. Add a byte-exact irongut fixture (badge line, pipe-less separator, Summary row, sticky-comment trailer) so the regression cannot reappear, and run the python tests in build-and-test.yaml, which previously only ran actionlint and the shell tests. --- .github/actions/sort-coverage-table/sort.py | 51 ++++++++++++++++----- .github/workflows/build-and-test.yaml | 3 ++ CHANGELOG.md | 4 ++ test/sort_coverage_table_test.py | 42 +++++++++++++++++ 4 files changed, 88 insertions(+), 12 deletions(-) diff --git a/.github/actions/sort-coverage-table/sort.py b/.github/actions/sort-coverage-table/sort.py index cece873..08c71cf 100644 --- a/.github/actions/sort-coverage-table/sort.py +++ b/.github/actions/sort-coverage-table/sort.py @@ -1,32 +1,59 @@ # Copyright (c) 2026 Peaceful Studio OÜ # SPDX-License-Identifier: Apache-2.0 +SEPARATOR_CHARSET = set('-: \t') + + +def _cells(line: str) -> list[str]: + stripped = line.strip() + if '|' not in stripped: + return [] + return stripped.strip('|').split('|') + + +def _is_table_row(line: str) -> bool: + return len(_cells(line)) >= 2 + + +def _is_separator_row(line: str) -> bool: + cells = _cells(line) + return len(cells) >= 2 and all(cell.strip() and set(cell) <= SEPARATOR_CHARSET for cell in cells) + + +def _is_table_start(lines: list[str], i: int) -> bool: + return ( + _is_table_row(lines[i]) + and not _is_separator_row(lines[i]) + and i + 1 < len(lines) + and _is_separator_row(lines[i + 1]) + ) + + +def _first_cell(row: str) -> str: + return _cells(row)[0].strip() + def sort_coverage_table(text: str) -> str: - first_cell = lambda row: row.split('|')[1].strip() lines = text.splitlines(keepends=True) result, i, sorted_first = [], 0, False while i < len(lines): - line = lines[i] - if not sorted_first and line.startswith('|'): - result.append(line) - i += 1 - if i < len(lines) and lines[i].startswith('|'): - result.append(lines[i]) - i += 1 + if not sorted_first and _is_table_start(lines, i): + result.append(lines[i]) + result.append(lines[i + 1]) + i += 2 data, summary = [], [] - while i < len(lines) and lines[i].startswith('|'): - if first_cell(lines[i]).startswith('**'): + while i < len(lines) and _is_table_row(lines[i]): + if _first_cell(lines[i]).startswith('**'): summary.append(lines[i]) else: data.append(lines[i]) i += 1 - data.sort(key=lambda r: first_cell(r).lower()) + data.sort(key=lambda row: _first_cell(row).lower()) result.extend(data) result.extend(summary) sorted_first = True else: - result.append(line) + result.append(lines[i]) i += 1 return ''.join(result) diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml index b6f7d9d..5b3778b 100644 --- a/.github/workflows/build-and-test.yaml +++ b/.github/workflows/build-and-test.yaml @@ -78,3 +78,6 @@ jobs: set -euo pipefail bash test/push-nuget_test.sh bash test/normalize-ci-matrix_test.sh + + - name: Run python tests + run: python3 -m unittest discover -s test -p '*_test.py' diff --git a/CHANGELOG.md b/CHANGELOG.md index b4afff0..2f85423 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Fix the coverage-table sort being a silent no-op for C# coverage comments — `irongut/CodeCoverageSummary` `format: markdown` emits tables without leading pipes, which the `sort-coverage-table` action did not recognize as tables; it now detects a header line followed by a `---` separator line with or without leading pipes. (#18) + ## [2.0.0] - 2026-06-12 **Migration:** reference workflows and actions at `@v2` (e.g. `peacefulstudio/github-actions/.github/workflows/csharp-ci.yaml@v2`). The floating `v1` tag is frozen at the v1.5.x state and will no longer advance. diff --git a/test/sort_coverage_table_test.py b/test/sort_coverage_table_test.py index e4c8a14..fc2ce38 100644 --- a/test/sort_coverage_table_test.py +++ b/test/sort_coverage_table_test.py @@ -84,6 +84,48 @@ def test_real_irongut_four_column_output(self): self.assertEqual(lines[5], "| com.example.zoo | 90% | N/A | ✔ |") self.assertEqual(lines[6], "| **Summary** | **83%** | **N/A** | |") + def test_irongut_csharp_markdown_without_leading_pipes_sorts_rows(self): + text = ( + "![Code Coverage](https://img.shields.io/badge/Code%20Coverage-85%25-success?style=flat)\n" + "\n" + "Package | Line Rate | Branch Rate | Complexity | Health\n" + "-------- | --------- | ----------- | ---------- | ------\n" + "Daml.Codegen.CSharp | 82% | 94% | 934 | ✔\n" + "Canton.LedgerApi | 91% | 89% | 412 | ✔\n" + "Daml.Codegen.Abstractions | 78% | 95% | 215 | ✔\n" + "**Summary** | **85%** (3942 / 4628) | **93%** (1375 / 1474) | **1561** | ✔\n" + "\n" + "\n" + ) + expected = ( + "![Code Coverage](https://img.shields.io/badge/Code%20Coverage-85%25-success?style=flat)\n" + "\n" + "Package | Line Rate | Branch Rate | Complexity | Health\n" + "-------- | --------- | ----------- | ---------- | ------\n" + "Canton.LedgerApi | 91% | 89% | 412 | ✔\n" + "Daml.Codegen.Abstractions | 78% | 95% | 215 | ✔\n" + "Daml.Codegen.CSharp | 82% | 94% | 934 | ✔\n" + "**Summary** | **85%** (3942 / 4628) | **93%** (1375 / 1474) | **1561** | ✔\n" + "\n" + "\n" + ) + self.assertEqual(sort_coverage_table(text), expected) + + def test_without_leading_pipes_sort_is_case_insensitive(self): + text = ( + "Name | Line Rate\n" + "---- | ---------\n" + "zoo | 80%\n" + "Apple | 90%\n" + ) + lines = sort_coverage_table(text).splitlines() + self.assertEqual(lines[2], "Apple | 90%") + self.assertEqual(lines[3], "zoo | 80%") + + def test_header_without_separator_is_not_a_table(self): + text = "a | b\nplain text without pipes\n" + self.assertEqual(sort_coverage_table(text), text) + def test_go_fixture_with_details_section_unsorted(self): text = ( "![Code Coverage](https://img.shields.io/badge/Code%20Coverage-75%25-red)\n" From fda931c2837364e020870d32494a88fd6d6ea6b5 Mon Sep 17 00:00:00 2001 From: monsieurleberre Date: Fri, 12 Jun 2026 11:25:53 +0200 Subject: [PATCH 2/4] fix(csharp-ci): merge cobertura reports before summarizing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit irongut/CodeCoverageSummary concatenates multiple cobertura files but does not union-merge packages across them: with several test projects covering the same assemblies, every package appears multiple times with partial rates and the summary dilutes (observed ~85% -> 43% on daml-codegen-csharp-internal#311). Reinstate v1's known-good merge: expand tests-glob under globstar+nullglob, fail loud when nothing matches, and dotnet-coverage merge into coverage/merged.cobertura.xml, which irongut now reads (composed against working-directory with the v2-style '.'/empty-safe expression). dotnet-coverage 18.0.6 is pinned inline via an env var — the removed dotnet-coverage-version input is not re-added, keeping the v2 input contract unchanged for a patch release. --- .github/workflows/csharp-ci.yaml | 41 +++++++++++++++++++++----------- CHANGELOG.md | 1 + README.md | 5 +++- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/.github/workflows/csharp-ci.yaml b/.github/workflows/csharp-ci.yaml index a5e6b1b..29221f4 100644 --- a/.github/workflows/csharp-ci.yaml +++ b/.github/workflows/csharp-ci.yaml @@ -48,7 +48,7 @@ on: description: >- JSON array of runner labels for the build-and-test matrix. Examples: '["ubuntu-latest"]', '["ubuntu-latest","macos-latest","windows-latest"]'. - Coverage report generation (summary, PR comment, job summary) + Coverage report generation (merge, summary, PR comment, job summary) only runs on the `ubuntu-latest` shard; if your matrix excludes `ubuntu-latest`, no coverage report will be produced. Ignored when `build-matrix` is set. When both `os-list` and `build-matrix` are @@ -115,14 +115,14 @@ on: tests-glob: description: >- Glob (relative to working-directory) used to locate the per-project - `*.cobertura.xml` files emitted by MTP's coverage extension, both - to assert coverage files were produced and as the `filename` - passed to irongut/CodeCoverageSummary, which aggregates all - matching files into the coverage report. The default narrows to - `bin/Release/net*/` so that stale cobertura files left over in - source-controlled or scratch directories do not get picked up. - Adjust if your tests live outside a top-level `tests/` directory - or target a non-Release configuration. + `*.cobertura.xml` files emitted by MTP's coverage extension. The + matching files are union-merged into a single report + (`coverage/merged.cobertura.xml`) by `dotnet-coverage merge` + before irongut/CodeCoverageSummary summarizes it. The default + narrows to `bin/Release/net*/` so that stale cobertura files + left over in source-controlled or scratch directories do not get + merged. Adjust if your tests live outside a top-level `tests/` + directory or target a non-Release configuration. required: false type: string default: 'tests/**/bin/Release/net*/**/*.cobertura.xml' @@ -221,6 +221,14 @@ jobs: dotnet-version: ${{ inputs.dotnet-version }} global-json-file: ${{ inputs.dotnet-version == '' && ((inputs.working-directory == '.' || inputs.working-directory == '') && 'global.json' || format('{0}/global.json', inputs.working-directory)) || '' }} + - name: Install dotnet-coverage + if: ${{ matrix.coverage }} + env: + DOTNET_COVERAGE_VERSION: '18.0.6' + run: | + set -euo pipefail + dotnet tool update -g dotnet-coverage --version "$DOTNET_COVERAGE_VERSION" + - name: Restore env: GITHUB_USERNAME: ${{ github.actor }} @@ -249,12 +257,13 @@ jobs: fi dotnet "${args[@]}" - - name: Assert coverage files produced + - name: Merge per-project cobertura reports if: ${{ matrix.coverage }} env: TESTS_GLOB: ${{ inputs.tests-glob }} run: | set -euo pipefail + mkdir -p coverage shopt -s globstar nullglob # shellcheck disable=SC2206 # intentional word-split: $TESTS_GLOB is a glob pattern (sourced from inputs.tests-glob via env:), expanded under globstar+nullglob cobertura_files=($TESTS_GLOB) @@ -262,13 +271,17 @@ jobs: echo "::error::No .cobertura.xml files produced under '$TESTS_GLOB' — did MTP coverage extension run? See the 'Caller prerequisites' subsection under csharp-ci.yaml in the peacefulstudio/github-actions README." exit 1 fi - echo "Found ${#cobertura_files[@]} cobertura files: ${cobertura_files[*]}" + echo "Merging ${#cobertura_files[@]} cobertura files: ${cobertura_files[*]}" + dotnet-coverage merge \ + -o coverage/merged.cobertura.xml \ + -f cobertura \ + "${cobertura_files[@]}" - name: Code Coverage Summary Report if: ${{ matrix.coverage }} uses: irongut/CodeCoverageSummary@51cc3a756ddcd398d447c044c02cb6aa83fdae95 # v1.3.0 with: - filename: ${{ (inputs.working-directory == '.' || inputs.working-directory == '') && inputs.tests-glob || format('{0}/{1}', inputs.working-directory, inputs.tests-glob) }} + filename: ${{ (inputs.working-directory == '.' || inputs.working-directory == '') && 'coverage/merged.cobertura.xml' || format('{0}/coverage/merged.cobertura.xml', inputs.working-directory) }} badge: true format: markdown output: both @@ -293,7 +306,7 @@ jobs: run: | set -euo pipefail if [ ! -f code-coverage-results.md ]; then - echo "::error::code-coverage-results.md not produced by irongut/CodeCoverageSummary — check that tests-glob matches the cobertura files MTP produced" + echo "::error::code-coverage-results.md not produced by irongut/CodeCoverageSummary — check the filename input and the merged cobertura path" exit 1 fi printf '## %s\n\n' "$COVERAGE_TITLE" | cat - code-coverage-results.md > code-coverage-results.tmp @@ -313,7 +326,7 @@ jobs: run: | set -euo pipefail if [ ! -f code-coverage-results.md ]; then - echo "::error::code-coverage-results.md not produced by irongut/CodeCoverageSummary — check that tests-glob matches the cobertura files MTP produced" + echo "::error::code-coverage-results.md not produced by irongut/CodeCoverageSummary — check the filename input and the merged cobertura path" exit 1 fi cat code-coverage-results.md >> "$GITHUB_STEP_SUMMARY" diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f85423..eb5e820 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix the coverage-table sort being a silent no-op for C# coverage comments — `irongut/CodeCoverageSummary` `format: markdown` emits tables without leading pipes, which the `sort-coverage-table` action did not recognize as tables; it now detects a header line followed by a `---` separator line with or without leading pipes. (#18) +- Fix diluted C# coverage numbers when multiple test projects cover the same assemblies: `csharp-ci.yaml` again union-merges the per-project cobertura files (matched by `tests-glob`) into one report via the `dotnet-coverage` global tool (pinned to 18.0.6 inside the workflow — no input) before `irongut/CodeCoverageSummary` runs, instead of letting irongut concatenate them with duplicated, partial package rows. The input/secret contract is unchanged. ## [2.0.0] - 2026-06-12 diff --git a/README.md b/README.md index 07bc4c6..161c74a 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,10 @@ stay on a previous SHA / tag until you've migrated the items below. - **`Directory.Packages.props` pinning**: - `xunit.v3` — `3.2.2` - - `Microsoft.Testing.Extensions.CodeCoverage` — `18.0.6`. + - `Microsoft.Testing.Extensions.CodeCoverage` — `18.0.6`, matching the + `dotnet-coverage` global tool version the workflow pins for merging + the per-project cobertura reports into a single file before + summarizing — keep them aligned. See [`canton-ledger-api-csharp#79`](https://github.com/peacefulstudio/canton-ledger-api-csharp/pull/79) for the MTP 1.x / 2.x compatibility rationale: do not bump `CodeCoverage` past 18.0.x until `xunit.v3` ships an MTP 2.x build — From 4d7930589fc65af96413feeee2acf12c0f59925a Mon Sep 17 00:00:00 2001 From: monsieurleberre Date: Fri, 12 Jun 2026 11:43:23 +0200 Subject: [PATCH 3/4] =?UTF-8?q?fix(coverage):=20harden=20review=20findings?= =?UTF-8?q?=20=E2=80=94=20warn=20on=20no=20table,=20workspace-root=20merge?= =?UTF-8?q?d=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply multi-agent review findings on the two coverage fixes: - document inline why the cobertura merge must precede irongut (CodeCoverageSummary v1.3.0 concatenates instead of union-merging), so the step does not get dropped again like in v2.0.0 - write merged.cobertura.xml under $GITHUB_WORKSPACE so irongut gets a constant filename, dropping the working-directory ternary - reword the two ::error:: guards to caller-actionable causes (tests-glob / merge step) instead of a non-existent filename input - emit a ::warning:: from sort.py when no markdown table is detected, so a future irongut output drift cannot silently no-op again; new contains_table helper with unit tests - add a fixture matching the real go-ci document shape: pipe-less coverage table followed by a leading-pipe gocyclo details table --- .github/actions/sort-coverage-table/sort.py | 7 ++++ .github/workflows/csharp-ci.yaml | 11 +++--- test/sort_coverage_table_test.py | 42 ++++++++++++++++++++- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/.github/actions/sort-coverage-table/sort.py b/.github/actions/sort-coverage-table/sort.py index 08c71cf..bb7e176 100644 --- a/.github/actions/sort-coverage-table/sort.py +++ b/.github/actions/sort-coverage-table/sort.py @@ -33,6 +33,11 @@ def _first_cell(row: str) -> str: return _cells(row)[0].strip() +def contains_table(text: str) -> bool: + lines = text.splitlines(keepends=True) + return any(_is_table_start(lines, i) for i in range(len(lines))) + + def sort_coverage_table(text: str) -> str: lines = text.splitlines(keepends=True) result, i, sorted_first = [], 0, False @@ -62,6 +67,8 @@ def sort_coverage_table(text: str) -> str: import os with open('code-coverage-results.md') as f: text = f.read() + if not contains_table(text): + print('::warning::sort-coverage-table: no markdown table detected in code-coverage-results.md — output left unchanged') with open('code-coverage-results.md.tmp', 'w') as f: f.write(sort_coverage_table(text)) os.replace('code-coverage-results.md.tmp', 'code-coverage-results.md') diff --git a/.github/workflows/csharp-ci.yaml b/.github/workflows/csharp-ci.yaml index 29221f4..35f4f71 100644 --- a/.github/workflows/csharp-ci.yaml +++ b/.github/workflows/csharp-ci.yaml @@ -258,12 +258,13 @@ jobs: dotnet "${args[@]}" - name: Merge per-project cobertura reports + # irongut/CodeCoverageSummary v1.3.0 concatenates multiple cobertura files instead of union-merging them — duplicate package rows with diluted totals (see #18-adjacent fix, daml-codegen-csharp-internal#311) — so the files must be merged into one before it runs if: ${{ matrix.coverage }} env: TESTS_GLOB: ${{ inputs.tests-glob }} run: | set -euo pipefail - mkdir -p coverage + mkdir -p "$GITHUB_WORKSPACE/coverage" shopt -s globstar nullglob # shellcheck disable=SC2206 # intentional word-split: $TESTS_GLOB is a glob pattern (sourced from inputs.tests-glob via env:), expanded under globstar+nullglob cobertura_files=($TESTS_GLOB) @@ -273,7 +274,7 @@ jobs: fi echo "Merging ${#cobertura_files[@]} cobertura files: ${cobertura_files[*]}" dotnet-coverage merge \ - -o coverage/merged.cobertura.xml \ + -o "$GITHUB_WORKSPACE/coverage/merged.cobertura.xml" \ -f cobertura \ "${cobertura_files[@]}" @@ -281,7 +282,7 @@ jobs: if: ${{ matrix.coverage }} uses: irongut/CodeCoverageSummary@51cc3a756ddcd398d447c044c02cb6aa83fdae95 # v1.3.0 with: - filename: ${{ (inputs.working-directory == '.' || inputs.working-directory == '') && 'coverage/merged.cobertura.xml' || format('{0}/coverage/merged.cobertura.xml', inputs.working-directory) }} + filename: coverage/merged.cobertura.xml badge: true format: markdown output: both @@ -306,7 +307,7 @@ jobs: run: | set -euo pipefail if [ ! -f code-coverage-results.md ]; then - echo "::error::code-coverage-results.md not produced by irongut/CodeCoverageSummary — check the filename input and the merged cobertura path" + echo "::error::code-coverage-results.md not produced by irongut/CodeCoverageSummary — check that tests-glob matched valid cobertura files and that the merge step succeeded" exit 1 fi printf '## %s\n\n' "$COVERAGE_TITLE" | cat - code-coverage-results.md > code-coverage-results.tmp @@ -326,7 +327,7 @@ jobs: run: | set -euo pipefail if [ ! -f code-coverage-results.md ]; then - echo "::error::code-coverage-results.md not produced by irongut/CodeCoverageSummary — check the filename input and the merged cobertura path" + echo "::error::code-coverage-results.md not produced by irongut/CodeCoverageSummary — check that tests-glob matched valid cobertura files and that the merge step succeeded" exit 1 fi cat code-coverage-results.md >> "$GITHUB_STEP_SUMMARY" diff --git a/test/sort_coverage_table_test.py b/test/sort_coverage_table_test.py index fc2ce38..e5d154f 100644 --- a/test/sort_coverage_table_test.py +++ b/test/sort_coverage_table_test.py @@ -6,7 +6,7 @@ import unittest sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..', '.github', 'actions', 'sort-coverage-table')) -from sort import sort_coverage_table +from sort import contains_table, sort_coverage_table TABLE_HEADER = "| Name | Line Rate |\n| :-- | :-: |\n" @@ -152,6 +152,46 @@ def test_go_fixture_with_details_section_unsorted(self): self.assertEqual(lines[6], "| **Summary** | **75%** | **N/A** | **2.7** | |") self.assertEqual(lines[13], "| 15 | `zoo_func` | `zoo/zoo.go:42` | ✓ 80.0% |") self.assertEqual(lines[14], "| 5 | `apple_func` | `apple/apple.go:10` | ✗ 0.0% |") + def test_real_go_document_pipe_less_coverage_table_with_gocyclo_details(self): + text = ( + "![Code Coverage](https://img.shields.io/badge/Code%20Coverage-75%25-red)\n" + "\n" + "Package | Line Rate | Branch Rate | Complexity | Health\n" + "-------- | --------- | ----------- | ---------- | ------\n" + "github.com/example/zoo | 80% | N/A | 3.2 | ✔\n" + "github.com/example/apple | 70% | N/A | 2.1 | ✗\n" + "**Summary** | **75%** | **N/A** | **2.7** | ✗\n" + "\n" + "
\n" + "Cyclomatic complexity — top 10 production functions (average 2.7)\n" + "\n" + "| Complexity | Function | Location | Attended |\n" + "| ---------: | -------- | -------- | :------- |\n" + "| 15 | `zoo_func` | `zoo/zoo.go:42` | ✓ 80.0% |\n" + "| 5 | `apple_func` | `apple/apple.go:10` | ✗ 0.0% |\n" + "\n" + "
\n" + ) + lines = sort_coverage_table(text).splitlines() + self.assertEqual(lines[4], "github.com/example/apple | 70% | N/A | 2.1 | ✗") + self.assertEqual(lines[5], "github.com/example/zoo | 80% | N/A | 3.2 | ✔") + self.assertEqual(lines[6], "**Summary** | **75%** | **N/A** | **2.7** | ✗") + self.assertEqual(lines[13], "| 15 | `zoo_func` | `zoo/zoo.go:42` | ✓ 80.0% |") + self.assertEqual(lines[14], "| 5 | `apple_func` | `apple/apple.go:10` | ✗ 0.0% |") + + def test_contains_table_detects_pipe_less_irongut_table(self): + text = ( + "Package | Line Rate | Health\n" + "-------- | --------- | ------\n" + "github.com/example/zoo | 80% | ✔\n" + ) + self.assertTrue(contains_table(text)) + + def test_contains_table_detects_leading_pipe_table(self): + self.assertTrue(contains_table(TABLE_HEADER + "| Zoo | 80% |\n")) + + def test_contains_table_is_false_when_no_table_present(self): + self.assertFalse(contains_table("Just some text\na | b without a separator line\n")) if __name__ == '__main__': From 2ae69e15a379b6d9f886144a5d9253f4d9271bad Mon Sep 17 00:00:00 2001 From: monsieurleberre Date: Fri, 12 Jun 2026 12:01:04 +0200 Subject: [PATCH 4/4] feat(csharp-ci): resolve dotnet-coverage version from caller's Directory.Packages.props The workflow installed dotnet-coverage 18.0.6 while callers pin Microsoft.Testing.Extensions.CodeCoverage independently (e.g. daml-codegen-csharp-internal pins 18.8.0 in tests/Directory.Packages.props), so merge tool and in-test collector could drift apart. Following the v2.0.0 global.json philosophy, a new helper script scripts/resolve-dotnet-coverage-version.py recursively scans every Directory.Packages.props under working-directory for the Microsoft.Testing.Extensions.CodeCoverage pin (Include or Update) and prints the single literal version; it fails loud on a missing pin, conflicting versions (listing files), or an MSBuild-property version, and skips unparseable XML with a warning. The helpers checkout moves up to just after Setup .NET so the resolve step can run before the tool install; the sort step reuses the same checkout. The version bump is now a caller-side props change only. --- .github/workflows/csharp-ci.yaml | 26 +++-- CHANGELOG.md | 2 +- README.md | 11 +- scripts/resolve-dotnet-coverage-version.py | 65 ++++++++++++ test/resolve_dotnet_coverage_version_test.py | 105 +++++++++++++++++++ 5 files changed, 195 insertions(+), 14 deletions(-) create mode 100644 scripts/resolve-dotnet-coverage-version.py create mode 100644 test/resolve_dotnet_coverage_version_test.py diff --git a/.github/workflows/csharp-ci.yaml b/.github/workflows/csharp-ci.yaml index 35f4f71..42e2bd9 100644 --- a/.github/workflows/csharp-ci.yaml +++ b/.github/workflows/csharp-ci.yaml @@ -221,10 +221,26 @@ jobs: dotnet-version: ${{ inputs.dotnet-version }} global-json-file: ${{ inputs.dotnet-version == '' && ((inputs.working-directory == '.' || inputs.working-directory == '') && 'global.json' || format('{0}/global.json', inputs.working-directory)) || '' }} + - name: Checkout CI helpers + if: ${{ matrix.coverage }} + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + repository: peacefulstudio/github-actions + ref: ${{ job.workflow_sha }} + path: .github-actions-helpers + + - name: Resolve dotnet-coverage version + if: ${{ matrix.coverage }} + id: dotnet-coverage + run: | + set -euo pipefail + version=$(python3 "$GITHUB_WORKSPACE/.github-actions-helpers/scripts/resolve-dotnet-coverage-version.py" .) + echo "version=$version" >> "$GITHUB_OUTPUT" + - name: Install dotnet-coverage if: ${{ matrix.coverage }} env: - DOTNET_COVERAGE_VERSION: '18.0.6' + DOTNET_COVERAGE_VERSION: ${{ steps.dotnet-coverage.outputs.version }} run: | set -euo pipefail dotnet tool update -g dotnet-coverage --version "$DOTNET_COVERAGE_VERSION" @@ -287,14 +303,6 @@ jobs: format: markdown output: both - - name: Checkout CI helpers - if: ${{ matrix.coverage }} - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - repository: peacefulstudio/github-actions - ref: ${{ job.workflow_sha }} - path: .github-actions-helpers - - name: Sort coverage table alphabetically if: ${{ matrix.coverage }} uses: ./.github-actions-helpers/.github/actions/sort-coverage-table diff --git a/CHANGELOG.md b/CHANGELOG.md index eb5e820..3abadbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix the coverage-table sort being a silent no-op for C# coverage comments — `irongut/CodeCoverageSummary` `format: markdown` emits tables without leading pipes, which the `sort-coverage-table` action did not recognize as tables; it now detects a header line followed by a `---` separator line with or without leading pipes. (#18) -- Fix diluted C# coverage numbers when multiple test projects cover the same assemblies: `csharp-ci.yaml` again union-merges the per-project cobertura files (matched by `tests-glob`) into one report via the `dotnet-coverage` global tool (pinned to 18.0.6 inside the workflow — no input) before `irongut/CodeCoverageSummary` runs, instead of letting irongut concatenate them with duplicated, partial package rows. The input/secret contract is unchanged. +- Fix diluted C# coverage numbers when multiple test projects cover the same assemblies: `csharp-ci.yaml` again union-merges the per-project cobertura files (matched by `tests-glob`) into one report via the `dotnet-coverage` global tool (version resolved from the caller's `Directory.Packages.props` pin of `Microsoft.Testing.Extensions.CodeCoverage`, nested files under `working-directory` included — no input; a missing, non-literal, or conflicting pin fails loud) before `irongut/CodeCoverageSummary` runs, instead of letting irongut concatenate them with duplicated, partial package rows. The input/secret contract is unchanged. ## [2.0.0] - 2026-06-12 diff --git a/README.md b/README.md index 161c74a..9296bbd 100644 --- a/README.md +++ b/README.md @@ -178,10 +178,13 @@ stay on a previous SHA / tag until you've migrated the items below. - **`Directory.Packages.props` pinning**: - `xunit.v3` — `3.2.2` - - `Microsoft.Testing.Extensions.CodeCoverage` — `18.0.6`, matching the - `dotnet-coverage` global tool version the workflow pins for merging - the per-project cobertura reports into a single file before - summarizing — keep them aligned. + - `Microsoft.Testing.Extensions.CodeCoverage` — required: the workflow + resolves the `dotnet-coverage` merge-tool version from this pin + (scanning every `Directory.Packages.props` under `working-directory`, + nested files included), so the two are aligned automatically. A + missing pin, an MSBuild-property version, or conflicting versions + across files fails the coverage shard loud — like a missing + `global.json`. The version must be a literal (e.g. `18.8.0`). See [`canton-ledger-api-csharp#79`](https://github.com/peacefulstudio/canton-ledger-api-csharp/pull/79) for the MTP 1.x / 2.x compatibility rationale: do not bump `CodeCoverage` past 18.0.x until `xunit.v3` ships an MTP 2.x build — diff --git a/scripts/resolve-dotnet-coverage-version.py b/scripts/resolve-dotnet-coverage-version.py new file mode 100644 index 0000000..d8a9ac4 --- /dev/null +++ b/scripts/resolve-dotnet-coverage-version.py @@ -0,0 +1,65 @@ +#!/usr/bin/env python3 +# Copyright (c) 2026 Peaceful Studio OÜ +# SPDX-License-Identifier: Apache-2.0 +import sys +import xml.etree.ElementTree as ElementTree +from pathlib import Path + +PACKAGE_ID = 'Microsoft.Testing.Extensions.CodeCoverage' +PROPS_FILENAME = 'Directory.Packages.props' + + +class VersionResolutionError(Exception): + pass + + +def _pinned_versions(props_file: Path) -> list[str]: + try: + tree = ElementTree.parse(props_file) + except ElementTree.ParseError: + print( + f'::warning::resolve-dotnet-coverage-version: skipping unparseable XML file {props_file}', + file=sys.stderr, + ) + return [] + return [ + element.attrib['Version'] + for element in tree.iter('PackageVersion') + if (element.get('Include') or element.get('Update')) == PACKAGE_ID + and 'Version' in element.attrib + ] + + +def collect_pins(root_dir: str) -> list[tuple[Path, str]]: + return [ + (props_file, version) + for props_file in sorted(Path(root_dir).rglob(PROPS_FILENAME)) + for version in _pinned_versions(props_file) + ] + + +def resolve_version(root_dir: str) -> str: + pins = collect_pins(root_dir) + if not pins: + raise VersionResolutionError( + f"no {PACKAGE_ID} pin found in any {PROPS_FILENAME} under '{root_dir}' — " + f'pin it in {PROPS_FILENAME}; it is required for MTP to emit cobertura files at all' + ) + property_pins = [(path, version) for path, version in pins if '$(' in version] + if property_pins: + listing = ', '.join(f'{path}: {version}' for path, version in property_pins) + raise VersionResolutionError( + f'{PACKAGE_ID} version must be a literal, not an MSBuild property: {listing}' + ) + if len({version for _, version in pins}) > 1: + listing = ', '.join(f'{path}: {version}' for path, version in pins) + raise VersionResolutionError(f'conflicting {PACKAGE_ID} versions: {listing}') + return pins[0][1] + + +if __name__ == '__main__': + try: + print(resolve_version(sys.argv[1])) + except VersionResolutionError as error: + print(f'::error::{error}', file=sys.stderr) + sys.exit(1) diff --git a/test/resolve_dotnet_coverage_version_test.py b/test/resolve_dotnet_coverage_version_test.py new file mode 100644 index 0000000..83f651e --- /dev/null +++ b/test/resolve_dotnet_coverage_version_test.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python3 +# Copyright (c) 2026 Peaceful Studio OÜ +# SPDX-License-Identifier: Apache-2.0 +import contextlib +import importlib.util +import io +import os +import tempfile +import unittest + +SCRIPT_PATH = os.path.join( + os.path.dirname(__file__), '..', 'scripts', 'resolve-dotnet-coverage-version.py' +) +spec = importlib.util.spec_from_file_location('resolve_dotnet_coverage_version', SCRIPT_PATH) +resolve_module = importlib.util.module_from_spec(spec) +spec.loader.exec_module(resolve_module) + + +def write_props(root, relative_dir, content): + directory = os.path.join(root, relative_dir) + os.makedirs(directory, exist_ok=True) + path = os.path.normpath(os.path.join(directory, 'Directory.Packages.props')) + with open(path, 'w') as f: + f.write(content) + return path + + +def props_with_pin(version, attribute='Include'): + return ( + '\n' + ' \n' + ' \n' + f' \n' + ' \n' + '\n' + ) + + +PROPS_WITHOUT_PIN = ( + '\n' + ' \n' + ' \n' + ' \n' + '\n' +) + + +class TestResolveDotnetCoverageVersion(unittest.TestCase): + + def test_single_pin_in_nested_directory_resolves(self): + with tempfile.TemporaryDirectory() as root: + write_props(root, 'tests', props_with_pin('18.8.0')) + self.assertEqual(resolve_module.resolve_version(root), '18.8.0') + + def test_no_pin_anywhere_raises(self): + with tempfile.TemporaryDirectory() as root: + write_props(root, '.', PROPS_WITHOUT_PIN) + with self.assertRaises(resolve_module.VersionResolutionError) as ctx: + resolve_module.resolve_version(root) + self.assertIn('Microsoft.Testing.Extensions.CodeCoverage', str(ctx.exception)) + self.assertIn('Directory.Packages.props', str(ctx.exception)) + + def test_conflicting_versions_raise_listing_both_files(self): + with tempfile.TemporaryDirectory() as root: + first = write_props(root, '.', props_with_pin('18.0.6')) + second = write_props(root, 'tests', props_with_pin('18.8.0')) + with self.assertRaises(resolve_module.VersionResolutionError) as ctx: + resolve_module.resolve_version(root) + self.assertIn(first, str(ctx.exception)) + self.assertIn(second, str(ctx.exception)) + self.assertIn('18.0.6', str(ctx.exception)) + self.assertIn('18.8.0', str(ctx.exception)) + + def test_identical_pins_in_two_files_resolve(self): + with tempfile.TemporaryDirectory() as root: + write_props(root, '.', props_with_pin('18.8.0')) + write_props(root, 'tests', props_with_pin('18.8.0')) + self.assertEqual(resolve_module.resolve_version(root), '18.8.0') + + def test_update_attribute_resolves(self): + with tempfile.TemporaryDirectory() as root: + write_props(root, '.', props_with_pin('19.0.0', attribute='Update')) + self.assertEqual(resolve_module.resolve_version(root), '19.0.0') + + def test_msbuild_property_version_raises(self): + with tempfile.TemporaryDirectory() as root: + write_props(root, '.', props_with_pin('$(CodeCoverageVersion)')) + with self.assertRaises(resolve_module.VersionResolutionError) as ctx: + resolve_module.resolve_version(root) + self.assertIn('literal', str(ctx.exception)) + + def test_malformed_xml_is_skipped_with_warning(self): + with tempfile.TemporaryDirectory() as root: + malformed = write_props(root, 'broken', '