diff --git a/scripts/ci/opencode_review_surfaces.py b/scripts/ci/opencode_review_surfaces.py index 55756d1ea9..afb7ceb42e 100644 --- a/scripts/ci/opencode_review_surfaces.py +++ b/scripts/ci/opencode_review_surfaces.py @@ -218,6 +218,8 @@ def rust_api_symbols(source_root: Path | None, raw_paths: Sequence[str]) -> list if not candidate.is_file() or candidate.is_symlink(): continue text = candidate.read_text(encoding="utf-8", errors="replace") + if "pub" not in text: + continue for match in PUB_ITEM_RE.finditer(text): name = match.group("name") if name not in seen: diff --git a/tests/test_docs_only_pr_runner_admission.py b/tests/test_docs_only_pr_runner_admission.py index 49631d2a19..5b1bfad3f4 100644 --- a/tests/test_docs_only_pr_runner_admission.py +++ b/tests/test_docs_only_pr_runner_admission.py @@ -85,19 +85,67 @@ def _on_block(workflow: str) -> str: return match.group(1) -def test_gate_job_is_byte_identical_across_the_five_workflows_apart_from_if(): - """The `changed-scope` block must not drift between its five copies.""" +def _strip_if_condition(block: str) -> str: + """Drop the `if:` line and, for a folded/literal scalar, its continuation lines. + + A workflow's `if:` condition can span multiple lines (``if: >-`` or ``if: |`` + followed by more-indented continuation lines) rather than a single line. + Comparing gate copies must ignore the whole condition, not just its first + line, since each copy is allowed its own admission condition independent + of how many source lines that condition takes. + """ + kept: list[str] = [] + skip_indent: int | None = None + for line in block.splitlines(): + if skip_indent is not None: + indent = len(line) - len(line.lstrip(" ")) + if not line.strip() or indent > skip_indent: + continue + skip_indent = None + if line.strip().startswith("if:"): + skip_indent = len(line) - len(line.lstrip(" ")) + continue + kept.append(line) + return "\n".join(kept) + + +def test_gate_job_is_byte_identical_across_the_three_workflows_apart_from_if(): + """The `changed-scope` block must not drift between its three copies.""" normalized_blocks = set() for filename in GATE_WORKFLOWS: workflow = _read(filename) block = _top_level_job_block(workflow, "changed-scope") - normalized = "\n".join( - line for line in block.splitlines() if not line.strip().startswith("if:") - ) + normalized = _strip_if_condition(block) normalized_blocks.add(normalized) assert len(normalized_blocks) == 1, ( "changed-scope gate copies drifted; keep them byte-identical apart " - "from the single 'if:' line" + "from the 'if:' condition" + ) + + +def test_strip_if_condition_keeps_skipping_across_a_blank_continuation_line(): + """A blank line inside a folded/literal `if:` scalar must not end the skip. + + YAML's `if: >-`/`if: |` block scalars can carry a blank line as part of + the same condition; a blank line is not itself an "if:"-less, less- + indented line that should end the skip, and one falsely resetting + `skip_indent` would leave that scalar's later indented lines in the + normalized output, making an otherwise byte-identical body compare as + drifted. + """ + block = ( + " steps:\n" + " - name: example\n" + " if: >-\n" + " first line ||\n" + "\n" + " second line after a blank\n" + " runs-on: ubuntu-24.04\n" + ) + assert _strip_if_condition(block) == ( + " steps:\n" + " - name: example\n" + " runs-on: ubuntu-24.04" ) @@ -208,7 +256,7 @@ def test_codeql_pr_gates_analyze_head_at_step_level_not_job_level(): def test_each_gate_workflow_keeps_an_always_admitted_job(): """A fully-skipped run must conclude `success`, never `skipped`. - Every one of the five workflows needs at least one job with no `needs:` + Every one of the three workflows needs at least one job with no `needs:` and no needs-output-dependent `if:` -- the `changed-scope` job itself qualifies -- so a doc-only PR's run still has a job that runs and succeeds instead of every job skipping and the run itself reporting diff --git a/tests/test_hourly_review_repair_callers.py b/tests/test_hourly_review_repair_callers.py index 0b9a049771..1c0ba3cb3d 100644 --- a/tests/test_hourly_review_repair_callers.py +++ b/tests/test_hourly_review_repair_callers.py @@ -162,7 +162,7 @@ # independent files (fast-mlsirm, metering-billing-platform) had each # chosen minute 49 without knowing about the other. The consolidated # lookup makes that sharing explicit and still dispatches each - # repository exactly once per hour, via the matrix in + # repository exactly once per day, via the matrix in # dispatch-review-repair. "49 12 * * *": [ { diff --git a/tests/test_noema_orchestrator_workflow_contract.py b/tests/test_noema_orchestrator_workflow_contract.py index 628fa3cbc1..be115f4572 100644 --- a/tests/test_noema_orchestrator_workflow_contract.py +++ b/tests/test_noema_orchestrator_workflow_contract.py @@ -116,7 +116,7 @@ def test_noema_close_cleanup_selects_only_the_closed_pr_across_shared_display_ti status="$(printf '%s' "$url" | sed -E 's/.*status=([a-z_]+)&.*/\\1/')" jq --arg status "$status" '{workflow_runs: [.workflow_runs[] | select(.status == $status)]}' \\ "$FAKE_RUNS_FILE" -elif [[ "$*" == *"/pulls/"* ]]; then +elif [[ "$*" == "api repos/ContextualWisdomLab/demo/pulls/7" ]]; then printf '%s\n' "$*" >>"$FAKE_CALLS_FILE" printf '%s\n' "$FAKE_LIVE_PR_JSON" else @@ -266,8 +266,17 @@ def _run_stale_trigger_step( ).split(" run: |\n", 1)[1] ) fake_gh = tmp_path / "gh" + calls_file = tmp_path / "calls.txt" fake_gh.write_text( - f"#!/usr/bin/env bash\nset -euo pipefail\nprintf '%s' '{live_head}'\n", + "#!/usr/bin/env bash\n" + "set -euo pipefail\n" + f"printf '%s\\n' \"$*\" >>'{calls_file}'\n" + "if [[ \"$*\" == 'api repos/ContextualWisdomLab/example/pulls/7 --jq .head.sha' ]]; then\n" + f" printf '%s' '{live_head}'\n" + "else\n" + " echo 'unexpected gh invocation' >&2\n" + " exit 1\n" + "fi\n", encoding="utf-8", ) fake_gh.chmod(0o755) diff --git a/tests/test_opencode_review_surfaces.py b/tests/test_opencode_review_surfaces.py index 858ca513b0..08e54be97b 100644 --- a/tests/test_opencode_review_surfaces.py +++ b/tests/test_opencode_review_surfaces.py @@ -410,6 +410,26 @@ def test_rust_api_symbols_skip_missing_and_symlink_sources(tmp_path: Path) -> No ) +def test_rust_api_symbols_skips_regex_when_pub_absent( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Files with no 'pub' substring take the fast path and skip regex scanning.""" + source = tmp_path / "lib.rs" + source.write_text("fn private_helper() {}\n", encoding="utf-8") + call_count = 0 + real_pattern = surfaces.PUB_ITEM_RE + + class CountingPattern: + def finditer(self, text: str): + nonlocal call_count + call_count += 1 + return real_pattern.finditer(text) + + monkeypatch.setattr(surfaces, "PUB_ITEM_RE", CountingPattern()) + assert surfaces.rust_api_symbols(tmp_path, ["lib.rs"]) == [] + assert call_count == 0 + + def test_rust_api_symbols_replace_invalid_utf8(tmp_path: Path) -> None: """A malformed Rust text blob cannot abort review-surface publication.""" source = tmp_path / "lib.rs" diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 2e733ac9e9..fba0c385bb 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -29,6 +29,7 @@ def test_review_fix_caller_keeps_the_github_daily_recovery_slot() -> None: """Keep the GitHub review repair caller on its distributed daily slot.""" caller = _workflow_text(HOURLY_CALLER_WORKFLOW) assert 'cron: "23 7 * * *"' in caller + assert 'cron: "23 */2 * * *"' not in caller assert 'cron: "23 * * * *"' not in caller assert "uses: ./.github/workflows/pr-review-fix-scheduler.yml" in caller