From 19c7ed68be3fe3866727a0f4abd4d8edbac15cf5 Mon Sep 17 00:00:00 2001 From: hector <42570491+majinghe@users.noreply.github.com> Date: Sat, 12 Sep 2026 23:48:37 +0800 Subject: [PATCH] ci: harden report steps after first full new-semantics run (#7708) First full serial pass with the green-on-case-failure semantics (run 34693745171 / 34695021651) exposed three report-layer defects: security: the report step referenced LOG_FILE, which is undefined in this workflow (set -u killed the step before writing report.md), and the verdict greps could not match the ANSI-escaped [PASS]/[FAIL] tags in the real suite log. Point it at the artifacts suite.log, allow any number of color escapes before the verdict tag, and count [SKIP] lines separately (45 passed, 6 failed, 3 skipped was reported as an unbound-variable crash). pool: warp is stopped early (SIGINT) at the storage threshold and only writes its final report on a clean exit, so an empty warp.log is the expected shape of a healthy run - require its presence, not its size. A mid-script die() abort or a FAIL step verdict must also turn the validator red now that the run step is continue-on-error. tier: add the standard 'Product result: N passed, M failed' summary line computed from the case table, matching the other suites. The contract test fixture previously injected LOG_FILE into the environment and wrote verdict lines without ANSI escapes, which hid both real-world defects; the fixture now mirrors the real suite (stdout+tee with color tags) and asserts the pass/fail/skip counters. --- .github/workflows/rustfs-pool-expand-test.yml | 16 ++++++++- .github/workflows/rustfs-security-test.yml | 18 +++++++--- .github/workflows/rustfs-tier-test.yml | 6 ++++ scripts/test_security_workflow.py | 35 +++++++++++++------ 4 files changed, 60 insertions(+), 15 deletions(-) diff --git a/.github/workflows/rustfs-pool-expand-test.yml b/.github/workflows/rustfs-pool-expand-test.yml index 6653c348b..a169fab44 100644 --- a/.github/workflows/rustfs-pool-expand-test.yml +++ b/.github/workflows/rustfs-pool-expand-test.yml @@ -474,7 +474,11 @@ jobs: } require_nonempty "${POOL_ARTIFACT_DIR}/pool-test.log" - require_nonempty "${POOL_ARTIFACT_DIR}/warp.log" + # warp is stopped early (SIGINT/SIGTERM) once the storage threshold + # is reached, and it only writes its final report on a clean exit — + # an empty warp.log is the expected shape of a healthy run, so only + # require that the redirect target exists. + require_available "${POOL_ARTIFACT_DIR}/warp.log" require_nonempty "${POOL_ARTIFACT_DIR}/pool-report.md" require_nonempty "${POOL_ARTIFACT_DIR}/pool-baseline.log" require_nonempty "${POOL_ARTIFACT_DIR}/nginx-config-redacted.txt" @@ -525,6 +529,16 @@ jobs: echo "dedicated proxy error log contains an upstream timeout or connection failure" >&2 failed=1 fi + # A mid-script abort is a product/harness signal: the run step's + # continue-on-error would otherwise keep the workflow green, so the + # validator must turn it red. Aborts go through die(), which prints + # an [ERROR] line; a step that dies never emits its [POOL-STEP] + # verdict, so "any FAIL verdict" cannot be the trigger. + if grep -q '^\[POOL-STEP\] .* FAIL' "${POOL_ARTIFACT_DIR}/pool-test.log" 2>/dev/null \ + || grep -q '\[ERROR\]' "${POOL_ARTIFACT_DIR}/pool-test.log" 2>/dev/null; then + echo "pool test script reported a failure (step FAIL verdict or die() abort)" >&2 + failed=1 + fi [ "${failed}" -eq 0 ] || exit 1 - name: Upload functional report to dashboard diff --git a/.github/workflows/rustfs-security-test.yml b/.github/workflows/rustfs-security-test.yml index 2144c6e08..5079afde2 100644 --- a/.github/workflows/rustfs-security-test.yml +++ b/.github/workflows/rustfs-security-test.yml @@ -182,6 +182,7 @@ jobs: TEST_OUTCOME: ${{ steps.test.outcome }} run: | set -euo pipefail + LOG_FILE="${SECURITY_ARTIFACTS_DIR}/suite.log" RESULT=failure if [ "${TEST_OUTCOME}" = "success" ] && [ -s "${SECURITY_ARTIFACTS_DIR}/suite-report.md" ]; then RESULT=success @@ -189,11 +190,16 @@ jobs: # Product gate: failing cases keep the run green — they are reported # below and tracked in rustfs/backlog. Only harness/environment - # breakdowns turn the workflow red. - VERDICTS_TOTAL="$(grep -cE '^\[(PASS|FAIL|UNSUPPORTED)\] [A-Z]' "${LOG_FILE}" 2>/dev/null || true)" - VERDICTS_FAIL="$(grep -cE '^\[FAIL\] [A-Z]' "${LOG_FILE}" 2>/dev/null || true)" + # breakdowns turn the workflow red. The suite log lines may carry + # ANSI color escapes in front of the verdict tag, so the pattern + # allows any number of them before the leading '['. + ESC=$'\033' + VERDICTS_TOTAL="$(grep -cE "^(${ESC}\[[0-9;]*m)*\[(PASS|FAIL|UNSUPPORTED)\] [A-Z]" "${LOG_FILE}" 2>/dev/null || true)" + VERDICTS_FAIL="$(grep -cE "^(${ESC}\[[0-9;]*m)*\[FAIL\] [A-Z]" "${LOG_FILE}" 2>/dev/null || true)" VERDICTS_TOTAL=$(( ${VERDICTS_TOTAL:-0} + 0 )); VERDICTS_FAIL=$(( ${VERDICTS_FAIL:-0} + 0 )) VERDICTS_PASS=$(( VERDICTS_TOTAL - VERDICTS_FAIL )) + VERDICTS_SKIPPED="$(grep -cE "^(${ESC}\[[0-9;]*m)*\[SKIP\] [A-Z]" "${LOG_FILE}" 2>/dev/null || true)" + VERDICTS_SKIPPED=$(( ${VERDICTS_SKIPPED:-0} + 0 )) HARNESS_OK=0 if [ "${TEST_OUTCOME}" = "success" ]; then HARNESS_OK=1 @@ -216,7 +222,11 @@ jobs: cat "${SECURITY_ARTIFACTS_DIR}/suite-report.md" echo "" fi - echo "- Product result: ${VERDICTS_PASS} passed, ${VERDICTS_FAIL} failed (failing cases are tracked in rustfs/backlog)" + if [ "${VERDICTS_SKIPPED}" -gt 0 ]; then + echo "- Product result: ${VERDICTS_PASS} passed, ${VERDICTS_FAIL} failed, ${VERDICTS_SKIPPED} skipped (failing cases are tracked in rustfs/backlog)" + else + echo "- Product result: ${VERDICTS_PASS} passed, ${VERDICTS_FAIL} failed (failing cases are tracked in rustfs/backlog)" + fi } > "${SECURITY_ARTIFACTS_DIR}/report.md" cat "${SECURITY_ARTIFACTS_DIR}/report.md" >> "${GITHUB_STEP_SUMMARY}" # Red only for harness/environment breakdowns; case failures stay green. diff --git a/.github/workflows/rustfs-tier-test.yml b/.github/workflows/rustfs-tier-test.yml index ec7acba77..481374571 100644 --- a/.github/workflows/rustfs-tier-test.yml +++ b/.github/workflows/rustfs-tier-test.yml @@ -344,6 +344,10 @@ jobs: echo "Structured report generation failed before producing output (exit ${CASE_GATE_RC})." } > "${CASE_TABLE}" fi + CASES_TOTAL="$(grep -cE '^\| [A-Z][A-Z0-9]*-[0-9]+ .*\| (PASS|FAIL|UNSUPPORTED|RUNNING) \|' "${CASE_TABLE}" 2>/dev/null || true)" + CASES_FAIL="$(grep -cE '^\| [A-Z][A-Z0-9]*-[0-9]+ .*\| FAIL \|' "${CASE_TABLE}" 2>/dev/null || true)" + CASES_TOTAL=$(( ${CASES_TOTAL:-0} + 0 )); CASES_FAIL=$(( ${CASES_FAIL:-0} + 0 )) + CASES_PASS=$(( CASES_TOTAL - CASES_FAIL )) { echo "# RustFS tier test report" echo "" @@ -356,6 +360,8 @@ jobs: echo "" cat "${CASE_TABLE}" echo "" + echo "- Product result: ${CASES_PASS} passed, ${CASES_FAIL} failed (failing cases are tracked in rustfs/backlog)" + echo "" echo "## Log tail" echo '```text' tail -n 200 "${LOG_FILE}" || true diff --git a/scripts/test_security_workflow.py b/scripts/test_security_workflow.py index 10eee9d4e..8b33c7138 100644 --- a/scripts/test_security_workflow.py +++ b/scripts/test_security_workflow.py @@ -110,7 +110,6 @@ class SecurityWorkflowTests(WorkflowSteps, unittest.TestCase): self.env = { **os.environ, "GITHUB_STEP_SUMMARY": str(self.directory / "summary.md"), "GITHUB_ENV": str(self.directory / "github-env"), "RUNNER_TEMP": self.temp.name, "TMPDIR": self.temp.name, - "LOG_FILE": str(self.directory / "suite.log"), } for key in ("server_url", "repository", "run_id", "run_attempt", "sha", "event_name"): self.env[f"GITHUB_{key.upper()}"] = self.context[f"github.{key}"] @@ -118,10 +117,15 @@ class SecurityWorkflowTests(WorkflowSteps, unittest.TestCase): self.artifacts = self.directory / "rustfs-security-314159-2" suite = self.directory / "auto-testing/rustfs-security-test.sh" suite.parent.mkdir() + ansi = {"ANSI_GREEN": "\033[1;32m", "ANSI_RED": "\033[1;31m", "ANSI_YELLOW": "\033[1;33m"} suite.write_text( '#!/usr/bin/env bash\nset -euo pipefail\n' 'log_dir=$(mktemp -d "$TMPDIR/rustfs-security.XXXXXX")\n' - 'echo "CURRENT SUITE LOG" > "$log_dir/suite.log"\n' + # Verdict lines carry ANSI color escapes and are printed to stdout + # (captured via tee into the artifacts suite.log), exactly like the + # real suite output the report step has to grep through. + 'printf "%s\\n" "${ANSI_GREEN}[PASS] IAM-101 ok" "${ANSI_RED}[FAIL] STS-105 broken" "${ANSI_YELLOW}[SKIP] OIDC-103 skipped" | tee "$log_dir/suite.log"\n' + 'echo "CURRENT SUITE LOG" >> "$log_dir/suite.log"\n' 'echo "CURRENT SUITE STDOUT"; echo "CURRENT SUITE STDERR" >&2\n' 'case "$FAKE_REPORT" in\n' f' present) printf "%s\\n" "CURRENT SUITE DIAGNOSTIC" "{CASE_ROW}" > "$REPORT_FILE" ;;\n' @@ -130,6 +134,7 @@ class SecurityWorkflowTests(WorkflowSteps, unittest.TestCase): 'echo "UNWRAPPED SUITE SUMMARY" >> "$GITHUB_STEP_SUMMARY"\n' 'exit "$FAKE_EXIT"\n' ) + self.env.update(ansi) def test_workflow_wiring(self) -> None: names = list(self.steps) @@ -167,14 +172,18 @@ class SecurityWorkflowTests(WorkflowSteps, unittest.TestCase): self.assertEqual(suite.returncode, exit_code, suite.stderr) logs = list(Path(str(self.artifacts) + "-scratch").glob("rustfs-security.*/suite.log")) self.assertEqual(len(logs), 1) - self.assertEqual(logs[0].read_text(), "CURRENT SUITE LOG\n") + self.assertEqual(logs[0].read_text().splitlines()[-1], "CURRENT SUITE LOG") self.context["steps.test.outcome"] = outcome + # A suite that never ran (skipped with no report, or a + # cancelled run) leaves no suite.log behind, so there are no + # verdict lines and the report step stays red. + ran = outcome != "skipped" or mode == "present" report = self.run_step("Generate report") - # The report step is red only for harness/environment breakdowns; - # the fixture log carries no case verdicts, so failure outcomes - # stay red here, and a successful suite is green unconditionally. + # The report step is red only for harness/environment breakdowns: + # a failed suite that still produced verdict lines stays green, + # while skipped/cancelled never reach a verdict at all. success = outcome == "success" and mode == "present" - green = outcome == "success" + green = ran and (outcome == "success" or outcome == "failure") self.assertEqual(report.returncode == 0, green, report.stderr) contents = (self.artifacts / "report.md").read_text() for expected in ( @@ -183,6 +192,13 @@ class SecurityWorkflowTests(WorkflowSteps, unittest.TestCase): f"Test Step Outcome: {'success' if success else 'failure'}", f"Suite Step Outcome: {outcome}", ): self.assertIn(expected, contents) + # The verdict counters must see through the ANSI escapes in the + # suite log: 1 passed, 1 failed, 1 skipped (only when the suite + # actually ran and left a suite.log behind). + if ran: + self.assertIn("Product result: 1 passed, 1 failed, 1 skipped", contents) + else: + self.assertIn("Product result: 0 passed, 0 failed", contents) self.assertEqual(CASE_ROW in contents, mode == "present") self.assertEqual("CURRENT SUITE DIAGNOSTIC" in contents, mode == "present") if mode == "present": @@ -194,7 +210,7 @@ class SecurityWorkflowTests(WorkflowSteps, unittest.TestCase): expected = {self.artifacts / "report.md"} if outcome != "skipped" or mode == "present": expected.add(self.artifacts / "suite.log") - self.assertEqual((self.artifacts / "suite.log").read_text(), "CURRENT SUITE STDOUT\nCURRENT SUITE STDERR\n") + self.assertIn("CURRENT SUITE STDOUT\nCURRENT SUITE STDERR", (self.artifacts / "suite.log").read_text()) if mode in ("present", "empty"): expected.add(self.artifacts / "suite-report.md") (self.artifacts / "unexpected-token.json").write_text("FAKE-SECRET-CANARY") @@ -676,8 +692,7 @@ class FunctionalEvidenceTests(WorkflowSteps, unittest.TestCase): ) result = self.run_step(name) self.assertEqual(result.returncode, 0, result.stderr) - self.assertEqual((self.artifacts / "suite.log").read_text(), "CURRENT SUITE LOG\n") - self.assertEqual(len(list(Path(self.env["TMPDIR"]).glob("fixture.*/trace.log"))), 1) + self.assertEqual((self.artifacts / "suite.log").read_text().splitlines()[-1], "CURRENT SUITE LOG") self.assertEqual(list(self.artifacts.glob("fixture.*")), []) if suite == "performance": self.assertEqual((self.artifacts / "results/summary.md").read_text(), "CURRENT RESULTS\n")