diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6eab9596..de8fd682 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,6 +88,7 @@ jobs: run_container: ${{ steps.scope.outputs.run_container }} run_desktop: ${{ steps.scope.outputs.run_desktop }} run_suite: ${{ steps.scope.outputs.run_suite }} + run_providers: ${{ steps.scope.outputs.run_providers }} steps: - uses: actions/checkout@v7 with: @@ -228,7 +229,7 @@ jobs: run: bash utils/smoke_container.sh vidxp:ci provider-platform-smoke: - if: needs.scope.outputs.run_suite == 'true' + if: needs.scope.outputs.run_providers == 'true' needs: scope name: CPU providers (${{ matrix.os }}) runs-on: ${{ matrix.os }} @@ -293,13 +294,20 @@ jobs: PROVIDER_RESULT: ${{ needs.provider-platform-smoke.result }} SCOPE_RESULT: ${{ needs.scope.result }} VALIDATE_RESULT: ${{ needs.validate.result }} + EXPECT_DESKTOP: ${{ needs.scope.outputs.run_desktop }} + EXPECT_PROVIDERS: ${{ needs.scope.outputs.run_providers }} + EXPECT_VALIDATE: ${{ needs.scope.outputs.needs_python }} shell: bash run: | - [[ "$SCOPE_RESULT" == "success" ]] - for result in \ - "$DESKTOP_RESULT" \ - "$PROVIDER_RESULT" \ - "$VALIDATE_RESULT" - do - [[ "$result" == "success" || "$result" == "skipped" ]] - done + [[ "$SCOPE_RESULT" == "success" ]] || exit 1 + require_result() { + if [[ "$1" == "true" ]]; then + [[ "$2" == "success" ]] + else + [[ "$1" == "false" ]] || return 1 + [[ "$2" == "success" || "$2" == "skipped" ]] + fi + } + require_result "$EXPECT_DESKTOP" "$DESKTOP_RESULT" || exit 1 + require_result "$EXPECT_PROVIDERS" "$PROVIDER_RESULT" || exit 1 + require_result "$EXPECT_VALIDATE" "$VALIDATE_RESULT" || exit 1 diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 1fcb66dc..0d545b4f 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -128,6 +128,16 @@ boundaries require maintainer review. For a new capability, follow Run the smallest relevant checks while developing, then the checks that cover the changed boundary before opening a pull request. +PRs limited to Python files under `src/vidxp/benchmarks/` and the benchmark +tests listed in `utils/ci_scope.py` keep the full Python suite, lockfile, +upload-page, and Python package smoke checks. They skip container builds, +Desktop packaging, and the separate Windows provider checks. Documentation +can accompany these changes without widening the scope. Dependency files, +shared code, packaging, workflows, and unrecognized paths retain their normal +validation, including when they accompany benchmark changes. Forced release +validation is unaffected. The `validation/required` check requires every +selected job to succeed. + | Change | Minimum validation | |---|---| | Python logic or contracts | Targeted test file, Ruff, then the full Python test suite | diff --git a/tests/test_ci_scope.py b/tests/test_ci_scope.py index 7b80a1c7..bf7ab97b 100644 --- a/tests/test_ci_scope.py +++ b/tests/test_ci_scope.py @@ -11,19 +11,20 @@ def test_documentation_only_changes_skip_code_validation(self): run_suite=False, run_container=False, run_desktop=False, + run_providers=False, ), ) def test_tests_and_desktop_changes_skip_container_builds(self): self.assertEqual( classify(["tests/test_new_feature.py", "desktop/src/App.tsx"]), - Scope(run_suite=True, run_container=False, run_desktop=True), + Scope(run_suite=True, run_container=False, run_desktop=True, run_providers=True), ) def test_benchmark_changes_skip_product_artifact_builds(self): self.assertEqual( classify(["benchmarks/codex-mcp/scripts/setup.mjs"]), - Scope(run_suite=True, run_container=False, run_desktop=False), + Scope(run_suite=True, run_container=False, run_desktop=False, run_providers=True), ) def test_product_and_workflow_changes_validate_containers(self): @@ -64,7 +65,7 @@ def test_desktop_uses_stable_product_and_packaging_boundaries(self): def test_unknown_new_roots_default_to_full_validation(self): self.assertEqual( classify(["future-product/component.rs"]), - Scope(run_suite=True, run_container=True, run_desktop=True), + Scope(run_suite=True, run_container=True, run_desktop=True, run_providers=True), ) def test_release_candidates_defer_to_the_candidate_build(self): @@ -78,6 +79,7 @@ def test_release_candidates_defer_to_the_candidate_build(self): run_suite=False, run_container=False, run_desktop=False, + run_providers=False, ), ) @@ -92,6 +94,7 @@ def test_main_to_release_sync_does_not_rebuild_validated_main(self): run_suite=False, run_container=False, run_desktop=False, + run_providers=False, ), ) @@ -102,7 +105,50 @@ def test_forced_candidate_validation_ignores_pr_scope(self): force_validation=True, run_containers=True, ), - Scope(run_suite=True, run_container=True, run_desktop=False), + Scope(run_suite=True, run_container=True, run_desktop=False, run_providers=True), + ) + + def test_benchmark_python_keeps_suite_without_product_builds(self): + for paths in ( + ["src/vidxp/benchmarks/latency.py", "tests/test_benchmark_latency.py"], + ["./src/vidxp/benchmarks/cli.py", "docs/benchmarking/performance.md"], + ["tests/test_benchmark_cli.py"], + ): + with self.subTest(paths=paths): + self.assertEqual(classify(paths), Scope(True, False, False, False)) + + def test_benchmark_changes_do_not_hide_shared_or_unknown_changes(self): + for path in ( + "src/vidxp/core/runner.py", + "src/vidxp/cli.py", + "src/vidxp/benchmarks/requirements.txt", + "pyproject.toml", + "uv.lock", + "MANIFEST.in", + "utils/ci_scope.py", + ".github/workflows/future.yml", + "future-product/component.rs", + ): + with self.subTest(path=path): + self.assertEqual( + classify(["src/vidxp/benchmarks/latency.py", path]), + Scope(True, True, True, True), + ) + + def test_unrecognized_tests_keep_existing_validation(self): + self.assertEqual( + classify(["tests/test_new_benchmark_runtime.py"]), + Scope(True, False, True, True), + ) + + def test_forced_validation_overrides_benchmark_exemption(self): + self.assertEqual( + select_scope( + ["src/vidxp/benchmarks/latency.py"], + force_validation=True, + run_containers=True, + ), + Scope(True, True, False, True), ) diff --git a/utils/ci_scope.py b/utils/ci_scope.py index f774404d..45cd1967 100644 --- a/utils/ci_scope.py +++ b/utils/ci_scope.py @@ -11,6 +11,17 @@ class Scope: run_suite: bool run_container: bool run_desktop: bool + run_providers: bool + + +def _is_benchmark_python(path: str) -> bool: + # Dependencies and packaging files must keep product validation enabled. + return (path.startswith("src/vidxp/benchmarks/") and path.endswith(".py")) or path in { + "tests/test_benchmarks.py", + "tests/test_benchmark_cli.py", + "tests/test_benchmark_latency.py", + "tests/test_benchmark_prepare.py", + } def _normalize(path: str) -> str: @@ -33,6 +44,8 @@ def _is_container_neutral(path: str) -> bool: def _affects_desktop(path: str) -> bool: + if path.startswith(".github/workflows/"): + return True return path.startswith(("desktop/", "plugins/", "src/", "tests/", "utils/")) or path in { ".github/workflows/ci.yml", ".github/workflows/desktop.yml", @@ -71,13 +84,19 @@ def classify(changed_files: list[str] | tuple[str, ...]) -> Scope: for value in changed_files if (path := _normalize(value)) and not _is_documentation(path) ] + benchmark_only = bool(code_paths) and all( + _is_benchmark_python(path) for path in code_paths + ) return Scope( run_suite=bool(code_paths), - run_container=any(not _is_container_neutral(path) for path in code_paths), - run_desktop=any( + run_container=not benchmark_only and any( + not _is_container_neutral(path) for path in code_paths + ), + run_desktop=not benchmark_only and any( _affects_desktop(path) or _is_unknown_product_path(path) for path in code_paths ), + run_providers=bool(code_paths) and not benchmark_only, ) @@ -94,6 +113,7 @@ def select_scope( run_suite=True, run_container=run_containers, run_desktop=False, + run_providers=True, ) if head_ref.startswith("release-please--branches--") or ( base_ref == "release" and head_ref == "main" @@ -102,6 +122,7 @@ def select_scope( run_suite=False, run_container=False, run_desktop=False, + run_providers=False, ) return classify(changed_files) @@ -156,6 +177,7 @@ def main() -> None: output.write(f"run_suite={str(scope.run_suite).lower()}\n") output.write(f"run_container={str(scope.run_container).lower()}\n") output.write(f"run_desktop={str(scope.run_desktop).lower()}\n") + output.write(f"run_providers={str(scope.run_providers).lower()}\n") output.write(f"needs_python={str(needs_python).lower()}\n")