mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-25 21:20:00 +00:00
Retain bounded API shard failure identities beside timings
Publish failed and unfinished source-test names in the existing shard artifact without waiting for other workflow jobs. Keep logs, timings, test selection and failure exits unchanged; record unknown completion and omitted unsafe or excess identities explicitly. Pin privacy, bounds, pipefail and timing-refresh compatibility, and align the existing shard contract with current compile admission. Change-source: pulse-maintainer
This commit is contained in:
@@ -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 <seconds-file>.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]} <seconds-file>", 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
|
||||
|
||||
|
||||
|
||||
@@ -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 `<seconds-file>.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 <id>` 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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
Reference in New Issue
Block a user