Merge pull request #229 from storytold/sonarcloud/bin-exe-paths

sonarcloud: let shard tests find workspace binaries by their build path

.github/workflows/sonarcloud.yml

@@ -280,6 +280,9 @@ jobs:
           # shellcheck disable=SC1090 # generated by cargo-llvm-cov at runtime
           source <(cargo llvm-cov show-env --sh)
           cargo nextest archive --workspace "${features[@]}" --archive-file nextest-archive.tar.zst
+          # env!("CARGO_BIN_EXE_<name>") compiles this target directory's absolute path into
+          # every test that runs a workspace binary; the shards link it to their extracted copy.
+          cargo metadata --format-version 1 --no-deps | jq -r .target_directory > target-dir.txt
       - name: Record the toolchain that built the archive
         run: |
           rustc -vV | sed -n 's/^release: //p' > rustc-release.txt
@@ -289,6 +292,7 @@ jobs:
           path: |
             nextest-archive.tar.zst
             rustc-release.txt
+            target-dir.txt
           retention-days: 1
 
   coverage:
@@ -385,12 +389,36 @@ jobs:
         run: |
           python3 scripts/fetch-pdf-test-fonts.py "$RUNNER_TEMP/craft-pdf-test-fonts"
           echo "CRAFT_PDF_TEST_FONT_DIR=$RUNNER_TEMP/craft-pdf-test-fonts" >> "$GITHUB_ENV"
+      - name: Link the build's target directory to the extracted archive
+        # Tests that run a workspace binary through env!("CARGO_BIN_EXE_<name>") hold the build
+        # job's absolute target path, which nextest's NEXTEST_BIN_EXE_<name> remapping cannot
+        # change. nextest extracts the archive elsewhere, so without this link every such test
+        # fails with "No such file or directory" under coverage while it passes in the gate.
+        shell: bash
+        env:
+          NEXTEST_EXTRACT_DIR: ${{ runner.temp }}/nextest-archive
+        run: |
+          set -euo pipefail
+          target_dir="$(cat archive/target-dir.txt)"
+          if [[ "$target_dir" != /* || "$target_dir" == *$'\n'* || "$target_dir" == */../* ]]; then
+            echo "::error::archive/target-dir.txt must hold one absolute path, got '$target_dir'"
+            exit 1
+          fi
+          if [ -e "$target_dir" ] || [ -L "$target_dir" ]; then
+            echo "::error::$target_dir already exists, so it cannot be linked to the extracted archive"
+            exit 1
+          fi
+          mkdir -p "$(dirname "$target_dir")" "$NEXTEST_EXTRACT_DIR"
+          ln -s "$NEXTEST_EXTRACT_DIR/target" "$target_dir"
+          echo "$target_dir -> $NEXTEST_EXTRACT_DIR/target"
       - name: Run this shard's tests
         env:
           # Each instrumented binary writes its profile here; the scan job merges them.
           LLVM_PROFILE_FILE: ${{ github.workspace }}/profraw/shard-${{ matrix.shard }}-%p-%m.profraw
           SHARD: ${{ matrix.shard }}
           TOTAL_SHARDS: ${{ inputs.shards }}
+          # The previous step links the build's target directory here.
+          NEXTEST_EXTRACT_DIR: ${{ runner.temp }}/nextest-archive
         run: |
           mkdir -p profraw
           # nextest exits 100 when tests fail. A red test must not hide the static analysis, so
@@ -399,6 +427,7 @@ jobs:
           # shard that runs nothing would scan an empty coverage report.
           rc=0
           cargo nextest run --archive-file archive/nextest-archive.tar.zst \
+            --extract-to "$NEXTEST_EXTRACT_DIR" --extract-overwrite \
             --partition "count:$SHARD/$TOTAL_SHARDS" --no-fail-fast || rc=$?
           if [ "$rc" -ne 0 ] && [ "$rc" -ne 100 ]; then
             echo "::error::nextest could not run shard $SHARD (exit $rc)"

README.md

@@ -912,6 +912,9 @@ cannot read (for example one cut short by a test process that crashed while
 writing it) fails its shard and is named in an error annotation, rather than
 reaching the scan. Pass `shards: 1` to keep the single `cargo llvm-cov` job.
 Sharded runs skip doctests, because nextest does not run them.
+Each shard links the build job's target directory to its extracted copy of the
+archive, so tests that run a workspace binary through
+`env!("CARGO_BIN_EXE_<name>")` find it as they do in the gate.
 Ordinary test failures still allow coverage and static analysis to upload, but then
 fail the scan job. Missing shard status also fails closed; a green analysis job
 must not conceal failed tests. The single-shard Rust path is unchanged.

tests/test_sonarcloud_bin_exe_paths.py

@@ -0,0 +1,103 @@
+"""Tests that run a workspace binary must find it under sharded coverage too."""
+
+import os
+from pathlib import Path
+import shutil
+import subprocess
+import tempfile
+import unittest
+
+import yaml
+
+
+ROOT = Path(__file__).resolve().parents[1]
+LINK = "Link the build's target directory to the extracted archive"
+RUN = "Run this shard's tests"
+
+
+@unittest.skipUnless(shutil.which('bash'), 'requires bash')
+class BinExePathTests(unittest.TestCase):
+    def setUp(self):
+        workflow = yaml.load((ROOT / '.github/workflows/sonarcloud.yml').read_text(),
+                             Loader=yaml.BaseLoader)
+        self.build = workflow['jobs']['build']['steps']
+        self.shard = workflow['jobs']['coverage']['steps']
+        self.link = next(step for step in self.shard if step.get('name') == LINK)
+
+    def test_build_records_the_target_directory_cargo_compiled_in(self):
+        archive = next(step for step in self.build
+                       if step.get('name') == 'Build instrumented test archive')['run']
+        # Recorded in the same shell as show-env, so it is the directory cargo really built into.
+        self.assertLess(archive.index('source <(cargo llvm-cov show-env --sh)'),
+                        archive.index('cargo metadata --format-version 1 --no-deps'))
+        self.assertIn('jq -r .target_directory > target-dir.txt', archive)
+        upload = next(step for step in self.build
+                      if step.get('uses', '').startswith('actions/upload-artifact@'))
+        self.assertIn('target-dir.txt', upload['with']['path'].split())
+
+    def test_link_precedes_the_tests_and_nextest_extracts_where_it_points(self):
+        names = [step.get('name') for step in self.shard]
+        self.assertLess(names.index(LINK), names.index(RUN))
+        run = self.shard[names.index(RUN)]
+        self.assertEqual(self.link['env']['NEXTEST_EXTRACT_DIR'],
+                         '${{ runner.temp }}/nextest-archive')
+        self.assertEqual(run['env']['NEXTEST_EXTRACT_DIR'], self.link['env']['NEXTEST_EXTRACT_DIR'])
+        self.assertIn('--extract-to "$NEXTEST_EXTRACT_DIR" --extract-overwrite', run['run'])
+
+    def run_link(self, root, target_dir):
+        archive = root / 'workspace/archive'
+        archive.mkdir(parents=True, exist_ok=True)
+        (archive / 'target-dir.txt').write_text(f'{target_dir}\n')
+        extract = root / 'runner temp/nextest-archive'
+        result = subprocess.run(
+            ['bash', '--noprofile', '--norc', '-e', '-o', 'pipefail', '-c', self.link['run']],
+            cwd=root / 'workspace', capture_output=True, text=True,
+            env={**os.environ, 'NEXTEST_EXTRACT_DIR': str(extract)},
+        )
+        return result, extract
+
+    def test_the_compiled_in_binary_path_reaches_the_extracted_binary(self):
+        with tempfile.TemporaryDirectory(prefix='sonar bin exe ') as temp:
+            root = Path(temp)
+            target_dir = root / 'workspace/target/llvm-cov-target'
+            result, extract = self.run_link(root, target_dir)
+            self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+            # What nextest --extract-to then unpacks: the archive's target/ tree.
+            binary = extract / 'target/debug/app-cli'
+            binary.parent.mkdir(parents=True)
+            binary.write_text('#!/bin/sh\necho "app-cli ran with $*"\n')
+            binary.chmod(0o755)
+            # The path env!("CARGO_BIN_EXE_app-cli") compiled into the test binary.
+            ran = subprocess.run([str(target_dir / 'debug/app-cli'), '--help'],
+                                 capture_output=True, text=True, check=True)
+            self.assertEqual(ran.stdout, 'app-cli ran with --help\n')
+
+    def test_unusable_target_paths_fail_the_shard(self):
+        for target_dir in ('target/llvm-cov-target', '/work/app/../escape', ''):
+            with self.subTest(target_dir=target_dir), tempfile.TemporaryDirectory() as temp:
+                result, _ = self.run_link(Path(temp), target_dir)
+                self.assertNotEqual(result.returncode, 0)
+                self.assertIn('::error::archive/target-dir.txt must hold one absolute path',
+                              result.stdout)
+
+    def test_two_paths_fail_the_shard(self):
+        with tempfile.TemporaryDirectory() as temp:
+            result, _ = self.run_link(Path(temp), '/work/one\n/work/two')
+            self.assertNotEqual(result.returncode, 0)
+            self.assertIn('::error::archive/target-dir.txt must hold one absolute path',
+                          result.stdout)
+
+    def test_an_existing_target_directory_is_never_replaced(self):
+        with tempfile.TemporaryDirectory() as temp:
+            root = Path(temp)
+            target_dir = root / 'workspace/target'
+            target_dir.mkdir(parents=True)
+            (target_dir / 'keep').write_text('keep')
+            result, _ = self.run_link(root, target_dir)
+            self.assertNotEqual(result.returncode, 0)
+            self.assertIn('already exists', result.stdout)
+            self.assertEqual((target_dir / 'keep').read_text(), 'keep')
+
+
+if __name__ == '__main__':
+    unittest.main()