diff --git a/.github/scripts/record-internal-api-test-seconds.py b/.github/scripts/record-internal-api-test-seconds.py index 8006abb945..369bf5593f 100755 --- a/.github/scripts/record-internal-api-test-seconds.py +++ b/.github/scripts/record-internal-api-test-seconds.py @@ -17,6 +17,12 @@ holding the whole test binary's run time, the same shape as .github/scripts/internal-api-test-seconds.txt, so refresh-internal-api-test-seconds.py can rebuild the shard weights from CI. +A bounded .failures.json sidecar names failed and unfinished +source-level tests without test output or parameterised subtest labels. The +existing timing-artifact upload makes it readable as soon as the shard ends, +even while another workflow job is still running. It is an observation index, +not a passing verdict: absent package completion and omitted names are explicit. + The exit status is non-zero when any test or package failed, but the caller must still run with `set -o pipefail` so a `go test` failure that produced no JSON (for example a vet or build error) fails the step. @@ -25,10 +31,26 @@ JSON (for example a vet or build error) fails the step. from __future__ import annotations import json +import re import sys from collections import defaultdict +MAX_FAILURE_IDENTIFIERS = 256 +MAX_IDENTIFIER_LENGTH = 256 +SOURCE_TEST_NAME = re.compile(r"(?:Test|Fuzz|Example)[A-Za-z0-9_]*") + + +def bounded_identifiers(names: set[str]) -> dict[str, object]: + """Never export output, subtest parameters or unsafe source names.""" + safe = sorted(name for name in names + if len(name) <= MAX_IDENTIFIER_LENGTH + and SOURCE_TEST_NAME.fullmatch(name)) + retained = safe[:MAX_FAILURE_IDENTIFIERS] + return {"names": retained, "observed_count": len(names), + "omitted_count": len(names) - len(retained)} + + def main() -> int: if len(sys.argv) != 2: print(f"usage: {sys.argv[0]} ", file=sys.stderr) @@ -39,6 +61,10 @@ def main() -> int: seconds: dict[str, float] = {} package_seconds = None failed = False + failed_tests: set[str] = set() + package_terminal_action = None + package_terminal_count = 0 + non_json_lines = 0 out = sys.stdout for raw in sys.stdin: @@ -47,6 +73,7 @@ def main() -> int: except ValueError: event = None if not isinstance(event, dict): + non_json_lines += 1 out.write(raw) out.flush() continue @@ -62,6 +89,9 @@ def main() -> int: if text: out.write(text) out.flush() + if action in ("pass", "fail", "skip"): + package_terminal_action = action + package_terminal_count += 1 if action == "fail": failed = True if action in ("pass", "fail") and isinstance(event.get("Elapsed"), (int, float)): @@ -79,6 +109,7 @@ def main() -> int: lines = held.pop(top, []) if action == "fail": failed = True + failed_tests.add(top) out.write("".join(lines)) out.flush() elapsed = event.get("Elapsed") @@ -97,6 +128,23 @@ def main() -> int: for name, value in seconds.items(): handle.write(f"{name} {value:.2f}\n") + # Upload alongside the unchanged timing text; never include raw output or + # parameterised subtest names. The CI artifact supplies run/source binding. + index = { + "schema_version": 1, + "recorder_exit_code": 1 if failed else 0, + "failed_top_level_tests": bounded_identifiers(failed_tests), + "unfinished_top_level_tests": bounded_identifiers(running), + # Only the final package action matters; retain contrary earlier + # actions as a count instead of exporting an unbounded event stream. + "package_terminal_action": package_terminal_action, + "package_terminal_count": package_terminal_count, + "non_json_line_count": non_json_lines, + } + with open(sys.argv[1] + ".failures.json", "w", encoding="utf-8") as handle: + json.dump(index, handle, indent=2, sort_keys=True) + handle.write("\n") + return 1 if failed else 0 diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index b627add0e5..9bfbd8372e 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -214,6 +214,19 @@ which prints only package lines and the output of failing or unfinished tests (as plain `go test` does), exits non-zero on any failure behind `pipefail`, and writes each top-level test's seconds to an `internal-api-test-seconds-N` artifact that is uploaded even when the shard fails. +The same existing artifact includes a `.failures.json` observation +index, available when its shard ends rather than when every workflow job ends. +It records failed and unfinished top-level source-test identities separately, +known package terminal actions and non-JSON line counts. It exports no assertion +text, package output or parameterised subtest names. Each identity list is capped +at 256 ASCII source names of at most 256 characters, with observed/omitted counts; +missing package completion remains explicit. The artifact's run/source binding +must be reconciled before using the index. Empty lists alone are not a pass, +and this does not replace terminal logs, diagnose failures, grant a retry or +change test selection, exit status, race/vet coverage or required check names. +`test_api_failure_index.py` exercises failure/unfinished/package-only/unknown, +privacy and bounds controls; the existing Go shard contract verifies the index +beside unchanged timing output. `.github/scripts/refresh-internal-api-test-seconds.py --run ` downloads those artifacts with `gh run download`, takes each test's median across the given runs, and rewrites the weights file with `DEFAULT_WEIGHT` set to the mean diff --git a/scripts/installtests/build_release_assets_test.go b/scripts/installtests/build_release_assets_test.go index 1437275da1..4ffc3de0fd 100644 --- a/scripts/installtests/build_release_assets_test.go +++ b/scripts/installtests/build_release_assets_test.go @@ -4050,7 +4050,7 @@ func TestBackendAPIShardsKeepRequiredCheckExhaustive(t *testing.T) { } } for _, required := range []string{ - "needs: changes", + "needs: [changes, test-compile]", "fail-fast: false", // The list comes from the commit under test, so a new test cannot be // missed, and contiguous slices of go test's own order put it in @@ -4088,8 +4088,9 @@ func TestBackendAPIShardsKeepRequiredCheckExhaustive(t *testing.T) { if strings.Contains(shards, "sort") { t.Fatal("internal/api shards must keep go test's run order; sorting splits order-coupled tests") } - if strings.Contains(shards, "\n if:") { - t.Fatal("internal/api shards must expand for every change so the verdict never sees skipped shards") + if !strings.Contains(shards, "if: ${{ !cancelled() && needs.changes.result == 'success' }}") || + strings.Contains(shards, "\n if: needs.changes.outputs.code") { + t.Fatal("internal/api shards must expand for documentation-only changes and compilation failures") } verdict := workflowJobBlock(t, workflow, "backend-api-verdict") @@ -4120,7 +4121,7 @@ func TestBackendAPIShardsKeepRequiredCheckExhaustive(t *testing.T) { func assertInternalAPITimingRecorderKeepsFailuresVisible(t *testing.T) { t.Helper() recorder := repoFile(".github", "scripts", "record-internal-api-test-seconds.py") - record := func(events string) (string, string, error) { + record := func(events string) (string, string, map[string]any, error) { out := filepath.Join(t.TempDir(), "seconds.txt") cmd := exec.Command("python3", recorder, out) cmd.Stdin = strings.NewReader(events) @@ -4129,7 +4130,15 @@ func assertInternalAPITimingRecorderKeepsFailuresVisible(t *testing.T) { if readErr != nil { t.Fatalf("recorder wrote no seconds file: %v", readErr) } - return string(printed), string(written), err + indexBytes, readErr := os.ReadFile(out + ".failures.json") + if readErr != nil { + t.Fatalf("recorder wrote no failure identity index: %v", readErr) + } + var index map[string]any + if err := json.Unmarshal(indexBytes, &index); err != nil { + t.Fatalf("failure index is not JSON: %v", err) + } + return string(printed), string(written), index, err } const pkg = `"Package":"example/internal/api"` passing := strings.Join([]string{ @@ -4139,7 +4148,7 @@ func assertInternalAPITimingRecorderKeepsFailuresVisible(t *testing.T) { `{"Action":"output",` + pkg + `,"Output":"ok \texample/internal/api\t2.000s\n"}`, `{"Action":"pass",` + pkg + `,"Elapsed":2}`, }, "\n") + "\n" - printed, written, err := record(passing) + printed, written, index, err := record(passing) if err != nil { t.Fatalf("recorder must pass a passing run: %v", err) } @@ -4150,6 +4159,11 @@ func assertInternalAPITimingRecorderKeepsFailuresVisible(t *testing.T) { t.Fatalf("recorder seconds file = %q", written) } + if index["package_terminal_action"] != "pass" || + index["failed_top_level_tests"].(map[string]any)["observed_count"] != float64(0) { + t.Fatalf("passing failure identity index = %v", index) + } + failing := strings.Join([]string{ `{"Action":"run",` + pkg + `,"Test":"TestBroken"}`, `{"Action":"output",` + pkg + `,"Test":"TestBroken/case","Output":"broken detail\n"}`, @@ -4158,7 +4172,7 @@ func assertInternalAPITimingRecorderKeepsFailuresVisible(t *testing.T) { `{"Action":"run",` + pkg + `,"Test":"TestHung"}`, `{"Action":"output",` + pkg + `,"Test":"TestHung","Output":"panic: test timed out\n"}`, }, "\n") + "\n" - printed, written, err = record(failing) + printed, written, index, err = record(failing) if err == nil { t.Fatal("recorder must exit non-zero when a test fails or never finishes") } @@ -4170,6 +4184,19 @@ func assertInternalAPITimingRecorderKeepsFailuresVisible(t *testing.T) { if written != "TestBroken 0.75\n" { t.Fatalf("recorder must record only finished top-level tests, wrote %q", written) } + for field, name := range map[string]string{ + "failed_top_level_tests": "TestBroken", + "unfinished_top_level_tests": "TestHung", + } { + observation := index[field].(map[string]any) + names := observation["names"].([]any) + if len(names) != 1 || names[0] != name || observation["omitted_count"] != float64(0) { + t.Fatalf("failure index %s = %v", field, observation) + } + } + if index["package_terminal_action"] != nil { + t.Fatalf("unfinished stream must not manufacture package completion: %v", index) + } } // assertInternalAPIShardSelectionExhaustive runs the shard selector the way diff --git a/scripts/tests/test_api_failure_index.py b/scripts/tests/test_api_failure_index.py new file mode 100644 index 0000000000..0c4a81b288 --- /dev/null +++ b/scripts/tests/test_api_failure_index.py @@ -0,0 +1,181 @@ +#!/usr/bin/env python3 +"""Shard artifacts expose bounded source identities, never assertion payloads.""" + +import json +from pathlib import Path +import subprocess +import sys +import tempfile +import unittest + + +ROOT = Path(__file__).resolve().parents[2] +RECORDER = ROOT / '.github/scripts/record-internal-api-test-seconds.py' + + +def event(action, test=None, output=None, elapsed=None): + value = {'Action': action, 'Package': 'example/internal/api'} + if test is not None: + value['Test'] = test + if output is not None: + value['Output'] = output + if elapsed is not None: + value['Elapsed'] = elapsed + return json.dumps(value) + '\n' + + +class APIFailureIndexTest(unittest.TestCase): + def record(self, stream): + with tempfile.TemporaryDirectory() as directory: + seconds = Path(directory) / 'api-2.txt' + result = subprocess.run([sys.executable, str(RECORDER), str(seconds)], + input=stream, text=True, capture_output=True, + timeout=10) + raw = Path(str(seconds) + '.failures.json').read_text() + return result, seconds.read_text(), json.loads(raw), raw + + def test_pass_preserves_timing_and_quiet_output(self): + result, seconds, index, _ = self.record( + event('run', 'TestGood') + event('output', 'TestGood', 'private output\n') + + event('pass', 'TestGood', elapsed=1.25) + event('pass', elapsed=2)) + self.assertEqual(result.returncode, 0) + self.assertEqual(result.stdout, '') + self.assertEqual(seconds, '# package-seconds 2.00\nTestGood 1.25\n') + self.assertEqual(index['failed_top_level_tests'], + {'names': [], 'observed_count': 0, 'omitted_count': 0}) + self.assertEqual(index['package_terminal_action'], 'pass') + self.assertEqual(index['package_terminal_count'], 1) + + def test_failure_names_root_without_subtest_or_assertion_data(self): + secret = 'customer@example.invalid' + result, seconds, index, raw = self.record( + event('run', 'TestBroken') + + event('output', 'TestBroken/' + secret, secret + '\n') + + event('fail', 'TestBroken/' + secret, elapsed=0.2) + + event('fail', 'TestBroken', elapsed=0.3) + event('fail', elapsed=0.4)) + self.assertEqual(result.returncode, 1) + self.assertIn(secret, result.stdout) # Existing diagnostic log stays intact. + self.assertEqual(seconds, '# package-seconds 0.40\nTestBroken 0.30\n') + self.assertEqual(index['failed_top_level_tests']['names'], ['TestBroken']) + self.assertEqual(index['unfinished_top_level_tests']['names'], []) + self.assertEqual(index['package_terminal_action'], 'fail') + self.assertNotIn(secret, raw) + self.assertNotIn('Package', raw) + + def test_unfinished_root_is_not_a_completed_failure_or_pass(self): + result, seconds, index, raw = self.record( + event('run', 'TestInterrupted') + + event('output', 'TestInterrupted/private-label', 'private log\n')) + self.assertEqual(result.returncode, 1) + self.assertIn('TestInterrupted did not finish', result.stdout) + self.assertEqual(seconds, '') + self.assertEqual(index['failed_top_level_tests']['names'], []) + self.assertEqual(index['unfinished_top_level_tests']['names'], ['TestInterrupted']) + self.assertIsNone(index['package_terminal_action']) + self.assertEqual(index['package_terminal_count'], 0) + self.assertNotIn('private', raw) + + def test_package_only_failure_does_not_invent_a_test_identity(self): + result, _, index, raw = self.record( + event('output', output='private compiler diagnostic\n') + event('fail')) + self.assertEqual(result.returncode, 1) + self.assertIn('compiler diagnostic', result.stdout) + self.assertEqual(index['failed_top_level_tests']['names'], []) + self.assertEqual(index['package_terminal_action'], 'fail') + self.assertNotIn('diagnostic', raw) + + def test_non_json_and_empty_streams_remain_unknown(self): + for stream, count in [('', 0), ('private non-json line\n[]\n', 2)]: + with self.subTest(stream=bool(stream)): + result, _, index, raw = self.record(stream) + # The upstream Go exit still belongs to pipefail, not this index. + self.assertEqual(result.returncode, 0) + self.assertEqual(index['non_json_line_count'], count) + self.assertIsNone(index['package_terminal_action']) + self.assertNotIn('private', raw) + + def test_unsafe_and_long_source_names_are_counted_but_not_exported(self): + for name in ['TestUnsafe\n::error::private', 'TestUnsafe\x1b[31m', + 'TestÜnicode', 'not-a-source-test', 'Test' + 'A' * 257]: + with self.subTest(name=name[:12]): + result, _, index, raw = self.record(event('fail', name)) + self.assertEqual(result.returncode, 1) + self.assertEqual(index['failed_top_level_tests'], + {'names': [], 'observed_count': 1, 'omitted_count': 1}) + self.assertNotIn(name, raw) + + def test_lists_are_deduplicated_sorted_and_bounded_separately(self): + stream = ''.join(event('fail', f'TestFailed{i:04d}') for i in range(300, -1, -1)) + stream += event('fail', 'TestFailed0000') + stream += ''.join(event('run', f'TestPending{i:04d}') for i in range(300, -1, -1)) + result, _, index, raw = self.record(stream) + self.assertEqual(result.returncode, 1) + for field in ['failed_top_level_tests', 'unfinished_top_level_tests']: + value = index[field] + self.assertEqual(len(value['names']), 256) + self.assertEqual(value['observed_count'], 301) + self.assertEqual(value['omitted_count'], 45) + self.assertEqual(value['names'], sorted(set(value['names']))) + self.assertLess(len(raw.encode()), 150_000) + + def test_package_actions_preserve_contrary_observations_without_green_claim(self): + result, _, index, _ = self.record(event('fail') + event('pass')) + self.assertEqual(result.returncode, 1) + self.assertEqual(index['package_terminal_action'], 'pass') + self.assertEqual(index['package_terminal_count'], 2) + self.assertNotIn('success', index) + self.assertEqual(index['recorder_exit_code'], 1) + + def test_skip_and_supported_source_name_forms(self): + stream = ''.join(event('fail', name) for name in + ['Test', 'TestUnder_score123', 'Example', 'ExampleThing', 'FuzzThing']) + result, _, index, _ = self.record(stream + event('skip')) + self.assertEqual(result.returncode, 1) + self.assertEqual(index['failed_top_level_tests']['observed_count'], 5) + self.assertEqual(index['failed_top_level_tests']['omitted_count'], 0) + self.assertEqual(index['package_terminal_action'], 'skip') + + def test_sidecar_is_ignored_by_the_existing_timing_refresh(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + seconds = root / 'api-2.txt' + result = subprocess.run([sys.executable, str(RECORDER), str(seconds)], + input=event('pass', 'TestTimed', elapsed=1.25) + + event('pass', elapsed=2), + text=True, capture_output=True, timeout=10) + self.assertEqual(result.returncode, 0, result.stderr) + weights = root / 'weights.out' + result = subprocess.run( + [sys.executable, str(ROOT / '.github/scripts/refresh-internal-api-test-seconds.py'), + '--dir', str(root), '--output', str(weights)], + text=True, capture_output=True, timeout=10) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn('TestTimed 1.25', weights.read_text()) + self.assertNotIn('failures.json', result.stdout) + + def test_pipefail_preserves_an_upstream_failure_with_no_json(self): + with tempfile.TemporaryDirectory() as directory: + seconds = Path(directory) / 'api-2.txt' + result = subprocess.run( + ['bash', '-c', 'set -o pipefail; (printf "compile diagnostic\\n"; exit 17) | ' + 'python3 "$1" "$2"', 'recorder-proof', str(RECORDER), str(seconds)], + text=True, capture_output=True, timeout=10) + self.assertEqual(result.returncode, 17) + index = json.loads(Path(str(seconds) + '.failures.json').read_text()) + self.assertIsNone(index['package_terminal_action']) + self.assertEqual(index['recorder_exit_code'], 0) + self.assertEqual(index['non_json_line_count'], 1) + + def test_existing_upload_collects_sidecar_on_a_failed_shard(self): + workflow = (ROOT / '.github/workflows/build-and-test.yml').read_text() + job = workflow.split('\n backend-api:\n', 1)[1].split('\n backend-api-verdict:', 1)[0] + self.assertIn('| python3 .github/scripts/record-internal-api-test-seconds.py', job) + self.assertIn('if: always() && needs.changes.outputs.code ==', job) + self.assertIn('path: ${{ runner.temp }}/internal-api-test-seconds/', job) + self.assertIn('name: internal-api-test-seconds-${{ matrix.index }}', job) + self.assertIn('set -euo pipefail', job) + self.assertNotIn('continue-on-error:', job) + + +if __name__ == '__main__': + unittest.main()