diff --git a/.github/scripts/backend-tests-workflow-contract_test.py b/.github/scripts/backend-tests-workflow-contract_test.py new file mode 100644 index 0000000000..7e2163122a --- /dev/null +++ b/.github/scripts/backend-tests-workflow-contract_test.py @@ -0,0 +1,94 @@ +#!/usr/bin/env python3 +"""Contract tests for bounded backend test summaries and retained diagnostics.""" + +from pathlib import Path +import unittest + + +REPO_ROOT = Path(__file__).resolve().parents[2] +BACKEND_WORKFLOW = REPO_ROOT / ".github" / "workflows" / "backend-tests.yml" +LINT_WORKFLOW = REPO_ROOT / ".github" / "workflows" / "lint-action-pinning.yml" + + +def step_block(workflow: str, name: str) -> str: + marker = f" - name: {name}\n" + _, separator, remainder = workflow.partition(marker) + if not separator: + raise AssertionError(f"backend-tests.yml has no {name!r} step") + return remainder.partition("\n - name: ")[0] + + +class BackendTestsWorkflowContractTest(unittest.TestCase): + def test_generated_report_is_redirected_then_published_with_a_bound(self) -> None: + workflow = BACKEND_WORKFLOW.read_text(encoding="utf-8") + checkout = step_block(workflow, "Checkout Go test reporter") + generate = step_block(workflow, "Generate test report") + publish = step_block(workflow, "Publish bounded test report summary") + + temporary_summary = "$RUNNER_TEMP/backend-test-summary-${{ matrix.shard }}.md" + self.assertIn( + "uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10", + checkout, + ) + self.assertIn("repository: robherley/go-test-action", checkout) + self.assertIn( + "ref: 2f859e0c8769d755d3174eecb9af8f64660827f3", + checkout, + ) + self.assertIn("path: .github/vendor/go-test-action", checkout) + self.assertIn(f'report_summary="{temporary_summary}"', generate) + self.assertNotIn("${{ runner.temp }}", generate) + self.assertIn(': > "$report_summary"', generate) + self.assertIn('GITHUB_STEP_SUMMARY="$report_summary"', generate) + self.assertIn( + 'INPUT_FROMJSONFILE="$GITHUB_WORKSPACE/apps/backend/' + 'test-results-${{ matrix.shard }}.json"', + generate, + ) + self.assertIn( + 'INPUT_MODULEDIRECTORY="$GITHUB_WORKSPACE/apps/backend"', + generate, + ) + self.assertIn('INPUT_OMIT="successful"', generate) + self.assertIn( + 'node ".github/vendor/go-test-action/dist/index.js"', + generate, + ) + self.assertNotIn("uses: robherley/go-test-action", generate) + self.assertIn("if: always()", publish) + self.assertIn("python3 .github/scripts/bounded-step-summary.py", publish) + self.assertIn(f'--input "{temporary_summary}"', publish) + self.assertIn('--output "$GITHUB_STEP_SUMMARY"', publish) + self.assertIn("--diagnostics-label", publish) + self.assertIn("backend-test-results-${{ matrix.shard }}", publish) + self.assertIn("${{ github.run_id }}#artifacts", publish) + + def test_full_json_diagnostics_remain_in_the_named_artifact(self) -> None: + workflow = BACKEND_WORKFLOW.read_text(encoding="utf-8") + upload = step_block(workflow, "Upload test artifacts") + + self.assertIn("if: always()", upload) + self.assertIn("name: backend-test-results-${{ matrix.shard }}", upload) + self.assertIn("apps/backend/test-results-${{ matrix.shard }}.json", upload) + + def test_summary_writer_changes_run_the_backend_workflow(self) -> None: + workflow = BACKEND_WORKFLOW.read_text(encoding="utf-8") + detect = step_block(workflow, "Detect relevant changes") + + self.assertIn(".github/scripts/bounded-step-summary.py", detect) + + def test_bounded_summary_contract_runs_in_ci(self) -> None: + workflow = LINT_WORKFLOW.read_text(encoding="utf-8") + + self.assertIn( + "python3 .github/scripts/bounded-step-summary_test.py", + workflow, + ) + self.assertIn( + "python3 .github/scripts/backend-tests-workflow-contract_test.py", + workflow, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/scripts/bounded-step-summary.py b/.github/scripts/bounded-step-summary.py new file mode 100644 index 0000000000..fb92e7d52f --- /dev/null +++ b/.github/scripts/bounded-step-summary.py @@ -0,0 +1,77 @@ +#!/usr/bin/env python3 +"""Write generated Markdown within GitHub Actions' step-summary size limit.""" + +import argparse +import re +from pathlib import Path + + +FAILURE_IDENTITY = re.compile(r"
  • šŸ”“([^<]*)
    • ") + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser() + parser.add_argument("--input", type=Path, required=True) + parser.add_argument("--output", type=Path, required=True) + parser.add_argument("--diagnostics-label", required=True) + parser.add_argument("--diagnostics-url", required=True) + parser.add_argument("--max-bytes", type=int, default=983_040) + return parser.parse_args() + + +def bounded_summary( + source: bytes, + *, + diagnostics_label: str, + diagnostics_url: str, + max_bytes: int, +) -> bytes: + source_text = source.decode("utf-8") + if len(source) <= max_bytes: + return source + + notice = ( + "> [!WARNING]\n" + "> This step summary was truncated to stay below GitHub Actions' " + "1 MiB limit. Full diagnostics are available in the " + f"[{diagnostics_label}]({diagnostics_url}) artifact.\n\n" + ).encode("utf-8") + if len(notice) > max_bytes: + raise ValueError("max bytes is too small for the truncation notice") + + prefix_budget = max_bytes - len(notice) + failure_identities = FAILURE_IDENTITY.findall(source_text) + identity_section = b"" + if failure_identities: + identity_section = ( + "## Failed test identities\n\n" + + "\n".join(f"- `{identity}`" for identity in failure_identities) + + "\n\n" + ).encode("utf-8") + identity_section = ( + identity_section[: max(0, prefix_budget)] + .decode("utf-8", errors="ignore") + .encode("utf-8") + ) + + prefix_budget -= len(identity_section) + # The full source is valid UTF-8, so ignoring errors can only discard a + # partial code point at the byte boundary. + prefix = source[:prefix_budget].decode("utf-8", errors="ignore").encode("utf-8") + return notice + identity_section + prefix + + +def main() -> int: + args = parse_args() + output = bounded_summary( + args.input.read_bytes(), + diagnostics_label=args.diagnostics_label, + diagnostics_url=args.diagnostics_url, + max_bytes=args.max_bytes, + ) + args.output.write_bytes(output) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/scripts/bounded-step-summary_test.py b/.github/scripts/bounded-step-summary_test.py new file mode 100644 index 0000000000..4339008839 --- /dev/null +++ b/.github/scripts/bounded-step-summary_test.py @@ -0,0 +1,155 @@ +#!/usr/bin/env python3 +"""Behavior tests for the bounded GitHub Actions step-summary writer.""" + +from pathlib import Path +import subprocess +import tempfile +import unittest + + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPT = REPO_ROOT / ".github" / "scripts" / "bounded-step-summary.py" + + +class BoundedStepSummaryTest(unittest.TestCase): + def run_writer(self, summary: str, *, max_bytes: int = 983_040) -> bytes: + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + source = tmp_path / "generated-summary.md" + output = tmp_path / "github-step-summary.md" + source.write_text(summary, encoding="utf-8") + + result = subprocess.run( + [ + "python3", + str(SCRIPT), + "--input", + str(source), + "--output", + str(output), + "--diagnostics-label", + "backend-test-results-1", + "--diagnostics-url", + "https://github.example.test/kdlbs/kandev/actions/runs/123#artifacts", + "--max-bytes", + str(max_bytes), + ], + capture_output=True, + text=True, + check=False, + ) + + self.assertEqual(result.returncode, 0, result.stderr) + return output.read_bytes() + + def test_normal_summary_is_unchanged(self) -> None: + summary = "## šŸ“ Test results\n\nšŸ”“ `TestWorkspaceFailure` failed\n" + + self.assertEqual(self.run_writer(summary), summary.encode("utf-8")) + + def test_oversized_summary_is_bounded_explicit_and_utf8_safe(self) -> None: + summary = ( + "## šŸ“ Test results\n\n" + "šŸ”“ `TestWorkspaceFailure` failed\n\n" + "boundary-padding-12" + + "diagnostic=å¤±ę•—šŸ”„\n" * 60_000 + ) + source_bytes = summary.encode("utf-8") + self.assertGreater(len(source_bytes), 1_048_576) + + first = self.run_writer(summary) + second = self.run_writer(summary) + rendered = first.decode("utf-8") + + self.assertLessEqual(len(first), 983_040) + self.assertEqual(first, second) + self.assertIn("TestWorkspaceFailure", rendered) + self.assertIn("truncated", rendered.lower()) + self.assertIn("backend-test-results-1", rendered) + self.assertIn( + "https://github.example.test/kdlbs/kandev/actions/runs/123#artifacts", + rendered, + ) + + def test_oversized_summary_retains_later_failure_identity(self) -> None: + summary = ( + "

    šŸ“ Test results

    \n" + "
      " + "
    • šŸ”“TestEarlyFailure
      • " + "
    \n" + + "diagnostic=å¤±ę•—šŸ”„\n" * 200_000 + + "
      " + "
    • šŸ”“TestLaterFailure
      • " + "
    \n" + ) + + rendered = self.run_writer(summary).decode("utf-8") + + self.assertIn("TestEarlyFailure", rendered) + self.assertIn("TestLaterFailure", rendered) + + def test_invalid_utf8_input_is_rejected(self) -> None: + # Contract coverage for the writer's existing UTF-8 validation. + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + source = tmp_path / "generated-summary.md" + output = tmp_path / "github-step-summary.md" + source.write_bytes(b"valid prefix\xff") + + result = subprocess.run( + [ + "python3", + str(SCRIPT), + "--input", + str(source), + "--output", + str(output), + "--diagnostics-label", + "backend-test-results-1", + "--diagnostics-url", + "https://github.example.test/kdlbs/kandev/actions/runs/123#artifacts", + ], + capture_output=True, + text=True, + check=False, + ) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("UnicodeDecodeError", result.stderr) + + def test_impossibly_small_max_bytes_is_rejected(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + source = tmp_path / "generated-summary.md" + output = tmp_path / "github-step-summary.md" + source.write_text("oversized summary", encoding="utf-8") + + result = subprocess.run( + [ + "python3", + str(SCRIPT), + "--input", + str(source), + "--output", + str(output), + "--diagnostics-label", + "backend-test-results-1", + "--diagnostics-url", + "https://github.example.test/kdlbs/kandev/actions/runs/123#artifacts", + "--max-bytes", + "1", + ], + capture_output=True, + text=True, + check=False, + ) + + self.assertNotEqual(result.returncode, 0) + self.assertIn( + "max bytes is too small for the truncation notice", + result.stderr, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/workflows/backend-tests.yml b/.github/workflows/backend-tests.yml index 2879c169b8..c969f677f3 100644 --- a/.github/workflows/backend-tests.yml +++ b/.github/workflows/backend-tests.yml @@ -63,6 +63,7 @@ jobs: scripts/check-make-shells .github/workflows/backend-tests.yml .github/scripts/changed-paths.py + .github/scripts/bounded-step-summary.py !**/*.md !**/*.mdx !**/*.markdown @@ -317,12 +318,41 @@ jobs: end ' - - name: Generate test report - uses: robherley/go-test-action@2f859e0c8769d755d3174eecb9af8f64660827f3 # v1.1.0 + # Run the pinned reporter as a child process so its summary target can be + # redirected. A `uses:` step cannot override GITHUB_STEP_SUMMARY because + # the runner reapplies its own file-command environment to JS actions. + - name: Checkout Go test reporter + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 if: always() with: - fromJSONFile: apps/backend/test-results-${{ matrix.shard }}.json - omit: successful + repository: robherley/go-test-action + ref: 2f859e0c8769d755d3174eecb9af8f64660827f3 # v1.1.0 + path: .github/vendor/go-test-action + persist-credentials: false + + - name: Generate test report + if: always() + shell: bash + working-directory: ${{ github.workspace }} + run: | + report_summary="$RUNNER_TEMP/backend-test-summary-${{ matrix.shard }}.md" + : > "$report_summary" + GITHUB_STEP_SUMMARY="$report_summary" \ + INPUT_FROMJSONFILE="$GITHUB_WORKSPACE/apps/backend/test-results-${{ matrix.shard }}.json" \ + INPUT_MODULEDIRECTORY="$GITHUB_WORKSPACE/apps/backend" \ + INPUT_OMIT="successful" \ + node ".github/vendor/go-test-action/dist/index.js" + + - name: Publish bounded test report summary + if: always() + shell: bash + working-directory: ${{ github.workspace }} + run: | + python3 .github/scripts/bounded-step-summary.py \ + --input "$RUNNER_TEMP/backend-test-summary-${{ matrix.shard }}.md" \ + --output "$GITHUB_STEP_SUMMARY" \ + --diagnostics-label "backend-test-results-${{ matrix.shard }}" \ + --diagnostics-url "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}#artifacts" - name: Upload test artifacts uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 diff --git a/.github/workflows/lint-action-pinning.yml b/.github/workflows/lint-action-pinning.yml index ba03d137bf..d219fc4a8f 100644 --- a/.github/workflows/lint-action-pinning.yml +++ b/.github/workflows/lint-action-pinning.yml @@ -73,6 +73,12 @@ jobs: - name: Test E2E workflow contract run: python3 .github/scripts/e2e-tests-workflow-contract_test.py + - name: Test bounded step summary + run: python3 .github/scripts/bounded-step-summary_test.py + + - name: Test backend tests workflow contract + run: python3 .github/scripts/backend-tests-workflow-contract_test.py + - name: Test stale merge approval revocation workflow contract run: python3 .github/scripts/revoke-ready-to-merge-workflow-contract_test.py diff --git a/apps/backend/internal/auth/hostnames/resolver_test.go b/apps/backend/internal/auth/hostnames/resolver_test.go index 5e80fe1c4d..a8023ab425 100644 --- a/apps/backend/internal/auth/hostnames/resolver_test.go +++ b/apps/backend/internal/auth/hostnames/resolver_test.go @@ -438,8 +438,12 @@ func TestResolverWakeQueueMatchesLookupConcurrency(t *testing.T) { func TestResolverChecksEachUserOncePerResolve(t *testing.T) { cache := &memoryCache{entries: map[string]CacheEntry{}, set: make(chan struct{}, 1)} var settingsReads atomic.Int32 + thirdSettingsRead := make(chan struct{}) + var thirdSettingsReadOnce sync.Once resolver := NewResolver(cache, func(context.Context, string) (bool, error) { - settingsReads.Add(1) + if settingsReads.Add(1) == 3 { + thirdSettingsReadOnce.Do(func() { close(thirdSettingsRead) }) + } return true, nil }, nil, nil, nil) resolver.lookupAddr = func(context.Context, string) ([]string, error) { @@ -454,9 +458,9 @@ func TestResolverChecksEachUserOncePerResolve(t *testing.T) { []string{"192.0.2.10", "::ffff:192.0.2.10"}, ) select { - case <-cache.set: + case <-thirdSettingsRead: case <-time.After(time.Second): - t.Fatal("hostname lookup did not finish") + t.Fatal("resolver did not perform the publication settings read") } if got := settingsReads.Load(); got != 3 { t.Fatalf("settings reads = %d, want admission, lookup, and publication reads", got)