From be272b4dcaebc194c11ead687e3fe6d08a43a713 Mon Sep 17 00:00:00 2001 From: Steve Ahn Date: Sat, 29 Nov 2025 12:40:33 -0800 Subject: [PATCH 1/4] pre-commit adds airflowctl int check --- airflow-ctl/.pre-commit-config.yaml | 10 ++ .../prek/check_airflowctl_command_coverage.py | 166 ++++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100755 scripts/ci/prek/check_airflowctl_command_coverage.py diff --git a/airflow-ctl/.pre-commit-config.yaml b/airflow-ctl/.pre-commit-config.yaml index 00eada7f64579..a68ac971c985f 100644 --- a/airflow-ctl/.pre-commit-config.yaml +++ b/airflow-ctl/.pre-commit-config.yaml @@ -53,3 +53,13 @@ repos: ^src/airflowctl/ctl/cli_config.py$| ^src/airflowctl/api/operations.py$| ^src/airflowctl/ctl/commands/.*\.py$ + - id: check-airflowctl-command-coverage + name: Check airflowctl CLI command test coverage + entry: ../scripts/ci/prek/check_airflowctl_command_coverage.py + language: python + pass_filenames: false + files: + (?x) + ^src/airflowctl/api/operations.py$| + ^../airflow-ctl-tests/tests/airflowctl_tests/conftest.py$| + ^../scripts/ci/prek/check_airflowctl_command_coverage.py$ diff --git a/scripts/ci/prek/check_airflowctl_command_coverage.py b/scripts/ci/prek/check_airflowctl_command_coverage.py new file mode 100755 index 0000000000000..49caf28fda386 --- /dev/null +++ b/scripts/ci/prek/check_airflowctl_command_coverage.py @@ -0,0 +1,166 @@ +#!/usr/bin/env python +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# /// script +# requires-python = ">=3.10,<3.11" +# dependencies = [ +# "rich>=13.6.0", +# ] +# /// +""" +Check that all airflowctl CLI commands have integration test coverage by comparing commands from operations.py against test_commands in conftest.py. +""" + +from __future__ import annotations + +import ast +import re +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).parent.resolve())) +from common_prek_utils import AIRFLOW_ROOT_PATH, console + +OPERATIONS_FILE = AIRFLOW_ROOT_PATH / "airflow-ctl" / "src" / "airflowctl" / "api" / "operations.py" +CONFTEST_FILE = AIRFLOW_ROOT_PATH / "airflow-ctl-tests" / "tests" / "airflowctl_tests" / "conftest.py" + +# Operations excluded from CLI (see cli_config.py) +EXCLUDED_OPERATION_CLASSES = {"BaseOperations", "LoginOperations", "VersionOperations"} +EXCLUDED_METHODS = { + "__init__", + "__init_subclass__", + "error", + "_check_flag_and_exit_if_server_response_error", + "bulk", +} + +# Commands intentionally not tested - grouped by reason: +# - Assets: require specific asset state, DAG dependencies, or queued events +# - Backfill: require inter-command data passing (IDs from create) +# - DAGs: destructive (delete) or require specific error/warning/version state +# - Connections: create_defaults may conflict with test environment +EXCLUDED_COMMANDS = { + # Assets - require asset state/dependencies + "assets materialize", + "assets get", + "assets get-by-alias", + "assets list-by-alias", + "assets create-event", + "assets get-queued-events", + "assets get-dag-queued-events", + "assets get-dag-queued-event", + "assets delete-queued-events", + "assets delete-dag-queued-events", + "assets delete-queued-event", + # Backfill - require IDs from create command + "backfill cancel", + "backfill create", + "backfill create-dry-run", + "backfill get", + "backfill pause", + "backfill unpause", + # DAGs - destructive or require specific state + "dags delete", + "dags get-details", + "dags get-tags", + "dags get-import-error", + "dags list-import-errors", + "dags get-stats", + "dags get-version", + "dags list-version", + "dags list-warning", + "dags trigger", # covered by dagrun trigger + # Connections + "connections create-defaults", + "connections test", # requires server-side configuration (core.test_connection) +} + + +def parse_operations() -> dict[str, list[str]]: + """Parse operations.py to extract CLI commands grouped by operation class.""" + commands: dict[str, list[str]] = {} + + with open(OPERATIONS_FILE) as f: + tree = ast.parse(f.read(), filename=str(OPERATIONS_FILE)) + + for node in ast.walk(tree): + if isinstance(node, ast.ClassDef) and node.name.endswith("Operations"): + if node.name in EXCLUDED_OPERATION_CLASSES: + continue + + group_name = node.name.replace("Operations", "").lower() + commands[group_name] = [] + + for child in node.body: + if isinstance(child, ast.FunctionDef): + method_name = child.name + if method_name in EXCLUDED_METHODS or method_name.startswith("_"): + continue + subcommand = method_name.replace("_", "-") + commands[group_name].append(subcommand) + + return commands + + +def parse_tested_commands() -> set[str]: + """Parse conftest.py to extract tested commands from test_commands fixture.""" + tested: set[str] = set() + + with open(CONFTEST_FILE) as f: + content = f.read() + + # Match command patterns like "assets list", "connections create --..." + pattern = r'"([a-z]+(?:-[a-z]+)?\s+[a-z]+(?:-[a-z]+)?)' + for match in re.findall(pattern, content): + parts = match.split() + if len(parts) >= 2: + tested.add(f"{parts[0]} {parts[1]}") + + return tested + + +def main(): + available = parse_operations() + tested = parse_tested_commands() + + missing = [] + for group, subcommands in sorted(available.items()): + for subcommand in sorted(subcommands): + cmd = f"{group} {subcommand}" + if cmd not in tested and cmd not in EXCLUDED_COMMANDS: + missing.append(cmd) + + if missing: + console.print("[red]ERROR: Commands not covered by integration tests:[/]") + for cmd in missing: + console.print(f" [red]- {cmd}[/]") + console.print() + console.print("[yellow]Fix by either:[/]") + console.print(" 1. Add test to airflow-ctl-tests/tests/airflowctl_tests/conftest.py") + console.print(" 2. Add to EXCLUDED_COMMANDS in scripts/ci/prek/check_airflowctl_command_coverage.py") + sys.exit(1) + + total = sum(len(cmds) for cmds in available.values()) + console.print( + f"[green]All {total} CLI commands covered ({len(tested)} tested, {len(EXCLUDED_COMMANDS)} excluded)[/]" + ) + sys.exit(0) + + +if __name__ == "__main__": + main() From 8d67b652786458565f04187987804b901ae2fb4c Mon Sep 17 00:00:00 2001 From: Steve Ahn Date: Sat, 29 Nov 2025 12:52:35 -0800 Subject: [PATCH 2/4] cleanup --- scripts/ci/prek/check_airflowctl_command_coverage.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/scripts/ci/prek/check_airflowctl_command_coverage.py b/scripts/ci/prek/check_airflowctl_command_coverage.py index 49caf28fda386..7c9b7a80b5f82 100755 --- a/scripts/ci/prek/check_airflowctl_command_coverage.py +++ b/scripts/ci/prek/check_airflowctl_command_coverage.py @@ -50,10 +50,6 @@ } # Commands intentionally not tested - grouped by reason: -# - Assets: require specific asset state, DAG dependencies, or queued events -# - Backfill: require inter-command data passing (IDs from create) -# - DAGs: destructive (delete) or require specific error/warning/version state -# - Connections: create_defaults may conflict with test environment EXCLUDED_COMMANDS = { # Assets - require asset state/dependencies "assets materialize", @@ -92,7 +88,6 @@ def parse_operations() -> dict[str, list[str]]: - """Parse operations.py to extract CLI commands grouped by operation class.""" commands: dict[str, list[str]] = {} with open(OPERATIONS_FILE) as f: @@ -118,13 +113,12 @@ def parse_operations() -> dict[str, list[str]]: def parse_tested_commands() -> set[str]: - """Parse conftest.py to extract tested commands from test_commands fixture.""" tested: set[str] = set() with open(CONFTEST_FILE) as f: content = f.read() - # Match command patterns like "assets list", "connections create --..." + # Match command patterns like "assets list", "connections create --..."", etc. pattern = r'"([a-z]+(?:-[a-z]+)?\s+[a-z]+(?:-[a-z]+)?)' for match in re.findall(pattern, content): parts = match.split() @@ -151,8 +145,8 @@ def main(): console.print(f" [red]- {cmd}[/]") console.print() console.print("[yellow]Fix by either:[/]") - console.print(" 1. Add test to airflow-ctl-tests/tests/airflowctl_tests/conftest.py") - console.print(" 2. Add to EXCLUDED_COMMANDS in scripts/ci/prek/check_airflowctl_command_coverage.py") + console.print("1. Add test to airflow-ctl-tests/tests/airflowctl_tests/conftest.py") + console.print("2. Add to EXCLUDED_COMMANDS in scripts/ci/prek/check_airflowctl_command_coverage.py") sys.exit(1) total = sum(len(cmds) for cmds in available.values()) From bf65ac842cff500bbd7a0cab549ba080157db48c Mon Sep 17 00:00:00 2001 From: Steve Ahn Date: Sun, 30 Nov 2025 12:37:08 -0800 Subject: [PATCH 3/4] remove asset materialize & others --- .../tests/airflowctl_tests/conftest.py | 8 ++++ .../prek/check_airflowctl_command_coverage.py | 39 +++++++------------ 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/airflow-ctl-tests/tests/airflowctl_tests/conftest.py b/airflow-ctl-tests/tests/airflowctl_tests/conftest.py index e5ccff6c51dd4..82561aa6e8625 100644 --- a/airflow-ctl-tests/tests/airflowctl_tests/conftest.py +++ b/airflow-ctl-tests/tests/airflowctl_tests/conftest.py @@ -243,6 +243,8 @@ def test_commands(login_command, date_param): login_command, # Assets commands "assets list", + "assets get --asset-id=1", + "assets create-event --asset-id=1", # Backfill commands "backfill list", # Config commands @@ -263,6 +265,12 @@ def test_commands(login_command, date_param): # DAGs commands "dags list", "dags get --dag-id=example_bash_operator", + "dags get-details --dag-id=example_bash_operator", + "dags get-stats --dag-ids=example_bash_operator", + "dags get-version --dag-id=example_bash_operator --version-number=1", + "dags list-import-errors", + "dags list-version --dag-id=example_bash_operator", + "dags list-warning", "dags pause --dag-id=example_bash_operator", "dags unpause --dag-id=example_bash_operator", "dags update --dag-id=example_bash_operator --is-paused=false", diff --git a/scripts/ci/prek/check_airflowctl_command_coverage.py b/scripts/ci/prek/check_airflowctl_command_coverage.py index 7c9b7a80b5f82..90ebae0e779f5 100755 --- a/scripts/ci/prek/check_airflowctl_command_coverage.py +++ b/scripts/ci/prek/check_airflowctl_command_coverage.py @@ -49,41 +49,28 @@ "bulk", } -# Commands intentionally not tested - grouped by reason: EXCLUDED_COMMANDS = { - # Assets - require asset state/dependencies - "assets materialize", - "assets get", - "assets get-by-alias", - "assets list-by-alias", - "assets create-event", - "assets get-queued-events", - "assets get-dag-queued-events", - "assets get-dag-queued-event", - "assets delete-queued-events", "assets delete-dag-queued-events", "assets delete-queued-event", - # Backfill - require IDs from create command + "assets delete-queued-events", + "assets get-by-alias", + "assets get-dag-queued-event", + "assets get-dag-queued-events", + "assets get-queued-events", + "assets list-by-alias", + "assets materialize", "backfill cancel", "backfill create", "backfill create-dry-run", "backfill get", "backfill pause", "backfill unpause", - # DAGs - destructive or require specific state + "connections create-defaults", + "connections test", "dags delete", - "dags get-details", - "dags get-tags", "dags get-import-error", - "dags list-import-errors", - "dags get-stats", - "dags get-version", - "dags list-version", - "dags list-warning", - "dags trigger", # covered by dagrun trigger - # Connections - "connections create-defaults", - "connections test", # requires server-side configuration (core.test_connection) + "dags get-tags", + "dags trigger", } @@ -118,8 +105,8 @@ def parse_tested_commands() -> set[str]: with open(CONFTEST_FILE) as f: content = f.read() - # Match command patterns like "assets list", "connections create --..."", etc. - pattern = r'"([a-z]+(?:-[a-z]+)?\s+[a-z]+(?:-[a-z]+)?)' + # Match command patterns like "assets list", "dags list-import-errors", etc. + pattern = r'"([a-z]+(?:-[a-z]+)*\s+[a-z]+(?:-[a-z]+)*)' for match in re.findall(pattern, content): parts = match.split() if len(parts) >= 2: From a56c9de37eb200c4f539d9e8cedbd79a635ebc97 Mon Sep 17 00:00:00 2001 From: Steve Ahn Date: Sun, 30 Nov 2025 14:31:38 -0800 Subject: [PATCH 4/4] dags update param, mege conflict, cicd error --- airflow-ctl-tests/tests/airflowctl_tests/conftest.py | 2 ++ scripts/ci/prek/check_airflowctl_command_coverage.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/airflow-ctl-tests/tests/airflowctl_tests/conftest.py b/airflow-ctl-tests/tests/airflowctl_tests/conftest.py index 67c470f8ef388..5186e3e794ea0 100644 --- a/airflow-ctl-tests/tests/airflowctl_tests/conftest.py +++ b/airflow-ctl-tests/tests/airflowctl_tests/conftest.py @@ -277,6 +277,8 @@ def test_commands(login_command, date_param): "dags unpause --dag-id=example_bash_operator", # DAG Run commands f'dagrun get --dag-id=example_bash_operator --dag-run-id="manual__{date_param}"', + "dags update --dag-id=example_bash_operator --no-is-paused", + # DAG Run commands "dagrun list --dag-id example_bash_operator --state success --limit=1", # Jobs commands "jobs list", diff --git a/scripts/ci/prek/check_airflowctl_command_coverage.py b/scripts/ci/prek/check_airflowctl_command_coverage.py index 90ebae0e779f5..00f89c723b70d 100755 --- a/scripts/ci/prek/check_airflowctl_command_coverage.py +++ b/scripts/ci/prek/check_airflowctl_command_coverage.py @@ -70,7 +70,6 @@ "dags delete", "dags get-import-error", "dags get-tags", - "dags trigger", } @@ -106,7 +105,8 @@ def parse_tested_commands() -> set[str]: content = f.read() # Match command patterns like "assets list", "dags list-import-errors", etc. - pattern = r'"([a-z]+(?:-[a-z]+)*\s+[a-z]+(?:-[a-z]+)*)' + # Also handles f-strings like f"dagrun get..." or f'dagrun get...' + pattern = r'f?["\']([a-z]+(?:-[a-z]+)*\s+[a-z]+(?:-[a-z]+)*)' for match in re.findall(pattern, content): parts = match.split() if len(parts) >= 2: