diff --git a/.github/actions/sort-coverage-table/sort.py b/.github/actions/sort-coverage-table/sort.py index cece873..bb7e176 100644 --- a/.github/actions/sort-coverage-table/sort.py +++ b/.github/actions/sort-coverage-table/sort.py @@ -1,32 +1,64 @@ # 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 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: - 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) @@ -35,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/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/.github/workflows/csharp-ci.yaml b/.github/workflows/csharp-ci.yaml index a5e6b1b..42e2bd9 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,30 @@ 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: ${{ steps.dotnet-coverage.outputs.version }} + run: | + set -euo pipefail + dotnet tool update -g dotnet-coverage --version "$DOTNET_COVERAGE_VERSION" + - name: Restore env: GITHUB_USERNAME: ${{ github.actor }} @@ -249,12 +273,14 @@ jobs: fi dotnet "${args[@]}" - - name: Assert coverage files produced + - 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 "$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) @@ -262,25 +288,21 @@ 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 "$GITHUB_WORKSPACE/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: coverage/merged.cobertura.xml badge: true 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 @@ -293,7 +315,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 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 @@ -313,7 +335,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 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/CHANGELOG.md b/CHANGELOG.md index b4afff0..3abadbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ 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) +- 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 **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/README.md b/README.md index 07bc4c6..9296bbd 100644 --- a/README.md +++ b/README.md @@ -178,7 +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`. + - `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', '\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" @@ -110,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__':