diff --git a/airflow-core/.pre-commit-config.yaml b/airflow-core/.pre-commit-config.yaml index 591b27fd02134..08382f3934f43 100644 --- a/airflow-core/.pre-commit-config.yaml +++ b/airflow-core/.pre-commit-config.yaml @@ -205,8 +205,7 @@ repos: files: | (?x) ^src/airflow/ui/.*\.(js|ts|tsx|yaml|css|json)$| - ^src/airflow/api_fastapi/core_api/openapi/.*\.yaml$| - ^src/airflow/api_fastapi/auth/managers/simple/openapi/v1.*\.yaml$ + ^src/airflow/api_fastapi/core_api/openapi/.*\.yaml$ exclude: | (?x) ^src/airflow/ui/node-modules/.*| diff --git a/dev/breeze/doc/ci/04_selective_checks.md b/dev/breeze/doc/ci/04_selective_checks.md index f8e27582ee9b1..3d64275757c67 100644 --- a/dev/breeze/doc/ci/04_selective_checks.md +++ b/dev/breeze/doc/ci/04_selective_checks.md @@ -252,6 +252,7 @@ representative examples (file → effect): | `.github/workflows/codeql-analysis.yml` (non-test workflow) | **basic checks only** | non-test workflow → cannot affect tests (env-files carve-out) | | `scripts/ci/prek/check_*.py` (static-check hook) | CI image + static checks, **no full matrix** | prek hooks are static checks → `Prek files` carve-out | | the generated OpenAPI spec | **full matrix** | the API *contract* ripples to UI codegen + every client | +| `core_api/openapi/_private_ui.yaml` (UI-only spec) | UI compile/lint prek hooks, **no full matrix** | matches `UI OpenAPI files`; the UI codegen input must be type-checked, but the public contract is unchanged | | `chart/templates/...yaml` (on `main`) | `run_helm_tests` (+ PROD image) | matches `HELM_FILES`; Helm tests only on `main` | | `task-sdk/.../task_runner.py` or `airflow-core/tests/integration/otel/...` | the `otel` core integration | matches `OTEL_FILES`; the otel integration tests assert the span hierarchy task_runner emits | | `airflow-core/src/airflow/ui/...tsx` only | `run_ui_tests`, **no** unit tests | "only new-UI files" short-circuit skips Python unit tests | @@ -363,6 +364,11 @@ We have the following Groups of files for CI that determine which tests are run: * `Always test files` - Files that belong to "Always" run tests. * `API tests files` and `Codegen test files` - those are OpenAPI definition files that impact Open API specification and determine that we should run dedicated API tests. +* `UI OpenAPI files` - the OpenAPI spec yamls under `core_api/openapi/` and under the simple + auth manager's `openapi/` that are the inputs of the UI client codegen. Membership in this + group does not force full tests (a generated spec still does, via `Codegen test files`); it + only keeps the UI compile/lint prek hooks from being skipped + (see [Skipping prek hooks](#skipping-prek-hooks-static-checks)). * `Helm files` - change in those files impacts helm "rendering" tests - `chart` folder (which contains the chart sources and tests under `chart/tests/`). * `Build files` - change in the files indicates that we should run `upgrade to newer dependencies` - build dependencies in `pyproject.toml` and generated dependencies files in `generated` folder. @@ -372,7 +378,6 @@ We have the following Groups of files for CI that determine which tests are run: * `DOC files` - change in those files indicate that we should run documentation builds (both airflow sources and airflow documentation) * `UI files` - those are files for the new full React UI (useful to determine if UI tests should run) -* `WWW files` - those are files for the WWW part of our UI (useful to determine if UI tests should run) * `System test files` - those are the files that are part of system tests (system tests are not automatically run in our CI, but Airflow stakeholders are running the tests and expose dashboards for them at [System Test Dashbards](https://airflow.apache.org/ecosystem/#airflow-provider-system-test-dashboards) @@ -497,8 +502,11 @@ when some files are not changed. Those are the rules implemented: `mypy-task-sdk-integration-tests`, `mypy-docker-tests`, `mypy-kubernetes-tests`) * for each `shared/` workspace member, `mypy-shared-` is skipped when no file under `shared//` changed (enumerated at runtime) - * if no `UI files` changed - `ts-compile-format-lint-ui` check is skipped - * if no `WWW files` changed - `ts-compile-format-lint-www` check is skipped + * if neither `UI files` nor `UI OpenAPI files` changed - the `ts-compile-lint-ui` and + `ts-compile-lint-simple-auth-manager-ui` checks are skipped. The `UI OpenAPI files` group + covers the union of those hooks' own openapi `files:` triggers, so a spec-only change + (e.g. `_private_ui.yaml`) still runs them - otherwise a stale committed client can mask + type errors (see #68919) * if no `All Python files` changed - `flynt` check is skipped * if no `Helm files` changed - `lint-helm-chart` check is skipped * if no `Java SDK files` changed - `ktlint` check is skipped (it runs the java-sdk Gradle diff --git a/dev/breeze/src/airflow_breeze/utils/selective_checks.py b/dev/breeze/src/airflow_breeze/utils/selective_checks.py index 95fe83b0e3853..63ac975ea66d4 100644 --- a/dev/breeze/src/airflow_breeze/utils/selective_checks.py +++ b/dev/breeze/src/airflow_breeze/utils/selective_checks.py @@ -117,6 +117,7 @@ class FileGroupForCi(Enum): DOC_FILES = auto() TEXT_NON_DOC_FILES = auto() UI_FILES = auto() + UI_OPENAPI_FILES = auto() SYSTEM_TEST_FILES = auto() KUBERNETES_FILES = auto() TASK_SDK_FILES = auto() @@ -379,6 +380,17 @@ def __hash__(self): r"^airflow-core/src/airflow/ui/", r"^airflow-core/src/airflow/api_fastapi/auth/managers/simple/ui/", ], + # The OpenAPI spec yamls that are inputs of the UI client codegen. Must cover the UNION of + # the openapi `files:` triggers of `ts-compile-lint-ui` and + # `ts-compile-lint-simple-auth-manager-ui` in `airflow-core/.pre-commit-config.yaml` — + # selective checks skip the two hooks as one unit, so this group is a strict superset of + # the first hook's triggers; do not "re-sync" it down to a single hook. A spec-only change + # (e.g. `_private_ui.yaml`) must not skip those hooks, otherwise a stale committed client + # masks type errors in CI (https://github.com/apache/airflow/pull/68919). + FileGroupForCi.UI_OPENAPI_FILES: [ + r"^airflow-core/src/airflow/api_fastapi/core_api/openapi/.*\.yaml", + r"^airflow-core/src/airflow/api_fastapi/auth/managers/simple/openapi/.*\.yaml", + ], FileGroupForCi.KUBERNETES_FILES: [ r"^chart", r"^kubernetes-tests", @@ -1671,7 +1683,9 @@ def skip_prek_hooks(self) -> str: return ",".join(sorted(prek_hooks_to_skip)) if not ( self._matching_files(FileGroupForCi.UI_FILES, CI_FILE_GROUP_MATCHES) - or self._matching_files(FileGroupForCi.API_CODEGEN_FILES, CI_FILE_GROUP_MATCHES) + # An API_CODEGEN_FILES disjunct would be unreachable here — matching that group + # forces full_tests_needed, and skip_prek_hooks returns early above in that case. + or self._matching_files(FileGroupForCi.UI_OPENAPI_FILES, CI_FILE_GROUP_MATCHES) ): prek_hooks_to_skip.add("ts-compile-lint-ui") prek_hooks_to_skip.add("ts-compile-lint-simple-auth-manager-ui") diff --git a/dev/breeze/tests/test_selective_checks.py b/dev/breeze/tests/test_selective_checks.py index 1dac0e3dd8354..699111954d088 100644 --- a/dev/breeze/tests/test_selective_checks.py +++ b/dev/breeze/tests/test_selective_checks.py @@ -115,6 +115,18 @@ ALL_SKIPPED_COMMITS_BY_DEFAULT_ON_ALL_TESTS_NEEDED = "identity,update-uv-lock" +ALL_SKIPPED_COMMITS_IF_ONLY_UI_OPENAPI_CHANGED = ( + "check-provider-yaml-valid,check-ts-sdk-supervisor-schema,flynt,identity,ktlint," + "lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,mypy-airflow-ctl-tests," + "mypy-airflow-e2e-tests,mypy-dev,mypy-devel-common,mypy-docker-tests,mypy-helm-tests," + "mypy-kubernetes-tests,mypy-scripts,mypy-shared-configuration,mypy-shared-dagnode," + "mypy-shared-listeners,mypy-shared-logging,mypy-shared-module_loading," + "mypy-shared-observability,mypy-shared-plugins_manager,mypy-shared-providers_discovery," + "mypy-shared-secrets_backend,mypy-shared-secrets_masker,mypy-shared-serialization," + "mypy-shared-state,mypy-shared-template_rendering,mypy-shared-timezones,mypy-task-sdk," + "mypy-task-sdk-integration-tests,update-uv-lock" +) + ALL_SKIPPED_COMMITS_IF_NO_UI = ( "check-ts-sdk-supervisor-schema,identity,ktlint,mypy-airflow-core,mypy-airflow-ctl,mypy-airflow-ctl-tests,mypy-airflow-e2e-tests," "mypy-dev,mypy-devel-common,mypy-docker-tests,mypy-helm-tests,mypy-kubernetes-tests," @@ -2045,6 +2057,33 @@ def test_provider_yaml_check_not_skipped_when_check_scripts_change(files: tuple[ assert "check-provider-yaml-valid" not in skip_prek_hooks.split(",") +@pytest.mark.parametrize( + "files", + [ + pytest.param( + ("airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml",), + id="private UI spec changed", + ), + pytest.param( + ( + "airflow-core/src/airflow/api_fastapi/auth/managers/simple/openapi/v2-simple-auth-manager-generated.yaml", + ), + id="simple auth manager spec changed", + ), + ], +) +def test_ui_compile_hooks_not_skipped_when_ui_openapi_spec_changes(files: tuple[str, ...]): + stderr = SelectiveChecks( + files=files, + github_event=GithubEvents.PULL_REQUEST, + commit_ref=NEUTRAL_COMMIT, + default_branch="main", + ) + skip_prek_hooks = str(stderr).split("skip-prek-hooks=")[1].split("\n")[0] + assert "ts-compile-lint-ui" not in skip_prek_hooks.split(",") + assert "ts-compile-lint-simple-auth-manager-ui" not in skip_prek_hooks.split(",") + + @pytest.mark.parametrize( ("files", "expected_outputs"), [ @@ -2735,6 +2774,27 @@ def test_expected_output_push( }, id="OpenAPI spec change still forces the full matrix", ), + pytest.param( + ("airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml",), + { + # Rationale on the UI_OPENAPI_FILES group in selective_checks.py. One param per + # spec directory so each pattern of the group is pinned individually - with both + # files in one param, dropping either pattern would still pass via the other file. + "full-tests-needed": "false", + "skip-prek-hooks": ALL_SKIPPED_COMMITS_IF_ONLY_UI_OPENAPI_CHANGED, + }, + id="Private UI OpenAPI spec change runs the UI compile hooks without the full matrix", + ), + pytest.param( + ( + "airflow-core/src/airflow/api_fastapi/auth/managers/simple/openapi/v2-simple-auth-manager-generated.yaml", + ), + { + "full-tests-needed": "false", + "skip-prek-hooks": ALL_SKIPPED_COMMITS_IF_ONLY_UI_OPENAPI_CHANGED, + }, + id="Simple auth manager OpenAPI spec change runs the UI compile hooks", + ), pytest.param( ( "airflow-core/src/airflow/assets/",