diff --git a/.gitignore b/.gitignore index a4cd4f0..892c9ae 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,6 @@ .venv/ __pycache__/ *.py[cod] + +# Keep repository tooling visible when a user globally ignores YAML files. +!.pre-commit-config.yaml diff --git a/.husky/pre-commit b/.husky/pre-commit new file mode 100755 index 0000000..f283485 --- /dev/null +++ b/.husky/pre-commit @@ -0,0 +1,67 @@ +#!/usr/bin/env sh +# Single Git entry point for both project areas. +# +# Git honors one hook path, and Husky owns it so both project areas can run +# their checks. `pre-commit install` refuses to write a hook while +# core.hooksPath is set, so this script calls `pre-commit run` instead; that is +# the same staged-file run the installed hook performs. +# +# Each area only runs when it has staged changes, and a commit touching both +# directories runs both. Nothing is nested: pre-commit and lint-staged each own +# their stash for the length of their own step. +# +# The areas are independent, so a failure in one still runs the other: a commit +# spanning both reports every problem instead of only the first. + +status=0 +# Treat renames as a deletion plus an addition so moving a flow to a path that +# no longer matches *_flow.py cannot bypass inventory validation. +staged=$(git diff --cached --no-renames --name-only --diff-filter=ACMRD) +deleted=$(git diff --cached --no-renames --name-only --diff-filter=D) + +# Catch conflict markers and whitespace errors in staged changes for both areas. +git diff --cached --check || status=1 + +# Config that governs how every hook behaves (ruff settings, hook definitions, +# pinned tool versions): a change here can affect files that aren't otherwise +# staged, so it forces `--all-files` below instead of the default scoped run. +config_pattern='^(\.pre-commit-config\.yaml$|pyproject\.toml$|\.husky/pre-commit$|flows/requirements(-dev)?\.txt$)' + +if printf '%s\n' "$staged" | grep -Eq "\.py$|$config_pattern"; then + if [ -x flows/.venv/bin/pre-commit ]; then + pre_commit=flows/.venv/bin/pre-commit + elif command -v pre-commit >/dev/null 2>&1; then + pre_commit=pre-commit + else + echo "pre-commit not found. Install it with:" >&2 + echo " python -m pip install -r flows/requirements-dev.txt" >&2 + pre_commit="" + status=1 + fi + + if [ -n "$pre_commit" ]; then + # A deletion under flows/ leaves no staged file on disk for pre-commit's own + # staged-file scan to match, so `pre-commit run` would silently skip + # check-flows; run --all-files instead so discovery still sees the deletion. + if printf '%s\n' "$staged" | grep -Eq "$config_pattern" \ + || printf '%s\n' "$deleted" | grep -Eq '^flows/'; then + "$pre_commit" run --all-files || status=1 + # Discovery reads the working tree, where an untracked flow may exist. + # Check the index directly so the commit itself must retain a flow. + if printf '%s\n' "$deleted" | grep -Eq '^flows/.*_flow\.py$' \ + && ! git ls-files --cached | grep -Eq '^flows/.*_flow\.py$'; then + echo "At least one tracked *_flow.py file is required under flows/." >&2 + status=1 + fi + else + # `files:` patterns in .pre-commit-config.yaml keep this to flows/. + "$pre_commit" run || status=1 + fi + fi +fi + +if git diff --cached --name-only --diff-filter=ACMR | grep -Eq '^e2e/(tests|test-setup)/'; then + (cd e2e && npm run precommit) || status=1 +fi + +exit "$status" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..1820958 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,38 @@ +repos: + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.16.7 + hooks: + - id: ruff-check + args: [--fix] + files: ^flows/.*\.py$ + - id: ruff-format + files: ^flows/.*\.py$ + + - repo: local + hooks: + - id: check-flow-filenames + name: Validate tracked FlowSpec filenames + entry: python flows/scripts/check_flows.py --tracked-filenames-only + language: python + files: \.py$ + pass_filenames: false + + - id: check-flows + name: Validate Metaflow definitions + entry: python flows/scripts/check_flows.py + language: python + additional_dependencies: + - outerbounds==0.12.44 + files: ^flows/.*_flow\.py$ + + - id: test-flow-tools + name: Test flow utilities + entry: pytest + args: [flows/tests] + language: python + additional_dependencies: + - pytest==9.1.1 + - pyyaml + # Run checker tests only when checker behavior, tests, or tool pins change. + files: ^(flows/scripts/check_flows\.py|flows/tests/.*\.py|flows/requirements(-dev)?\.txt|\.pre-commit-config\.yaml|pyproject\.toml)$ + pass_filenames: false diff --git a/README.md b/README.md index d9fce32..0ee0c39 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,42 @@ The `e2e/` directory contains browser-based end-to-end tests built with Playwrig See [`e2e/tests/README.md`](e2e/tests/README.md) for the Playwright test structure, import conventions, and validation workflow. +## Development Checks + +Each project area keeps its own tooling: `flows/` uses the root-level +`pre-commit` configuration, and `e2e/` uses its existing npm pre-commit command. +Git honors a single hook path, so a Husky hook at [`.husky/pre-commit`](.husky/pre-commit) +is the entry point. Staged Python files activate FlowSpec filename validation; +files under `flows/` also activate the flow checks. Added, copied, modified, or +renamed files under `e2e/tests/` and `e2e/test-setup/` activate the Playwright +checks. Deletion-only e2e changes skip that staged-file pipeline. A commit +touching both scopes runs both. + +Install the Python development requirements and the hook from the repository +root: + +```bash +python -m pip install -r flows/requirements-dev.txt +npm --prefix e2e install +``` + +Do not run `pre-commit install`; it refuses to write a hook while Husky owns +`core.hooksPath`, and the Husky hook already invokes `pre-commit run`. + +Run every local check explicitly with: + +```bash +pre-commit run --all-files +npm --prefix e2e run quality:full +``` + +Staged `*_flow.py` changes run Ruff and native Metaflow definition checks; +checker tests run only when checker tooling or configuration changes. Staged +existing files under `e2e/tests/` and `e2e/test-setup/` are formatted and linted +before the Playwright quality report runs. The hook also rejects conflict +markers and whitespace errors in staged changes. No hook authenticates to +Outerbounds or starts remote workloads. + ## Repository Layout ```text diff --git a/e2e/.husky/pre-commit b/e2e/.husky/pre-commit deleted file mode 100644 index 60b037d..0000000 --- a/e2e/.husky/pre-commit +++ /dev/null @@ -1,4 +0,0 @@ -if git diff --cached --name-only --diff-filter=ACMR | grep -qE '^(tests|test-setup)/'; then - npm run precommit || exit 1 -fi - diff --git a/e2e/package.json b/e2e/package.json index c4c25a7..c0bf342 100644 --- a/e2e/package.json +++ b/e2e/package.json @@ -33,7 +33,7 @@ "trace": "playwright show-trace --port 0", "lint": "cross-env eslint 'tests/**/*.ts' 'test-setup/**/*.ts' 'playwright.config.ts'", "lint:fix": "cross-env eslint 'tests/**/*.ts' 'test-setup/**/*.ts' 'playwright.config.ts' --fix", - "prepare": "husky", + "prepare": "cd .. && husky", "format": "cross-env prettier --write --no-error-on-unmatched-pattern 'tests/**/*.ts' 'test-setup/**/*.ts' 'playwright.config.ts' '**/*.json' '**/*.md' '!package-lock.json' '!dist/**/*' '!build/**/*'", "postinstall": "playwright install chromium", "ncu:check": "npx npm-check-updates", diff --git a/flows/README.md b/flows/README.md index 409ae8c..4468fb2 100644 --- a/flows/README.md +++ b/flows/README.md @@ -7,19 +7,111 @@ independent Metaflow flows and their supporting test data and utilities. Each flow validates a focused scenario and can be run independently on an Outerbounds cluster. -### Setup +## Pre-Commit Setup -Use an Outerbounds-supported Python version and install the client package in -an isolated environment: +Run all setup commands from the repository root. Use an Outerbounds-supported +Python version to create an isolated environment: ```bash -cd flows -python3 -m venv .venv -source .venv/bin/activate +cd /path/to/ob-quality +python3 -m venv flows/.venv +source flows/.venv/bin/activate python -m pip install --upgrade pip -python -m pip install -r requirements.txt +python -m pip install -r flows/requirements-dev.txt ``` +`requirements-dev.txt` includes the runtime requirements, so a separate +installation of `flows/requirements.txt` is not required for development. + +Git honors a single hook path, and the repository gives it to Husky so the +Playwright checks keep their lint-staged auto-fixes. `.husky/pre-commit` calls +`pre-commit run` when a commit stages files under `flows/`, so do not run +`pre-commit install` — it refuses to write a hook while `core.hooksPath` is set. + +Install the shared hook from the repository root: + +```bash +npm --prefix e2e install +``` + +Confirm that Husky owns the hook path and that `pre-commit` is callable from it: + +```bash +test "$(git config --get core.hooksPath)" = ".husky/_" && echo "husky installed" +pre-commit --version +``` + +The hook falls back to `flows/.venv/bin/pre-commit` when the virtual environment +is not active, so a commit from an editor or GUI client still runs the flow +checks. + +### Test the Hook + +Run all flow hooks against all tracked files: + +```bash +pre-commit run --all-files +``` + +Run only the flow-related hooks: + +```bash +pre-commit run ruff-check --all-files +pre-commit run ruff-format --all-files +pre-commit run check-flows --all-files +pre-commit run test-flow-tools --all-files +``` + +The flow hooks perform these checks: + +- `ruff-check`: Python linting, import ordering, common bug checks, and public + method docstrings. +- `ruff-format`: Python formatting. +- `check-flow-filenames`: staged-index validation that every tracked Python file + containing a `FlowSpec` uses the `*_flow.py` suffix. +- `check-flows`: flow discovery, repository conventions, and native Metaflow + definition and DAG validation. +- `test-flow-tools`: unit tests for the custom checker and tool-version + consistency. + +During a normal `git commit`, staged files determine which hooks run. A staged +`*_flow.py` file under `flows/` activates Ruff and native Metaflow validation; +other staged Python files under `flows/` activate Ruff. Any staged Python file +activates repository-wide FlowSpec filename validation. Checker tests run only +when `check_flows.py`, its tests, requirements, or tool configuration changes. +The native Metaflow check runs only for staged flow files during normal runs, +while filename and deletion checks use the staged index. This catches rewritten +delete/add pairs without depending on Git rename detection and requires the +index to retain at least one tracked flow. + +Added, copied, modified, or renamed files under `e2e/tests/` and +`e2e/test-setup/` separately activate the Playwright format, lint, and quality +checks; deletion-only e2e changes skip that staged-file pipeline. A commit +touching both scopes runs both. Ruff also fixes staged Python lint and formatting +issues. All staged changes are checked for conflict markers and whitespace +errors. Neither project check authenticates to Outerbounds or starts a +Kubernetes workload. + +## Flow Validation + +Every executable QA flow must: + +- Use a `*_flow.py` filename. +- Retain the `*_flow.py` suffix when renamed or moved. +- Define exactly one top-level `FlowSpec` subclass. +- Use a flow class name that is unique across all domains. +- Instantiate that class under `if __name__ == "__main__"`. + +To validate only flow discovery and Metaflow definitions: + +```bash +python flows/scripts/check_flows.py +``` + +This command runs Metaflow's native `check` command for every discovered flow. +It validates definitions and DAG structure without authenticating, downloading +models, or starting local or remote flow runs. + Configure a Metaflow profile for the target Outerbounds cluster before running a flow. Remote task pods must receive `OBP_API_SERVER`, `OBP_PERIMETER`, and `METAFLOW_SERVICE_HEADERS`; the platform normally injects these values. diff --git a/flows/models/browse_models_flow.py b/flows/models/browse_models_flow.py index e4e7e4f..563c260 100644 --- a/flows/models/browse_models_flow.py +++ b/flows/models/browse_models_flow.py @@ -5,7 +5,6 @@ """ from metaflow import FlowSpec, anaconda_models, step - from testdata.model_catalog_data import BROWSE_LIMIT from utils.model_validators import validate_models @@ -14,11 +13,10 @@ class BrowseModelsFlow(FlowSpec): @anaconda_models @step def start(self): + """Browse the model catalog and validate returned metadata.""" models = self.anaconda_models.list_models(limit=BROWSE_LIMIT) - assert isinstance(models, list), ( - f"Expected a list of models, got {type(models).__name__}" - ) + assert isinstance(models, list), f"Expected a list of models, got {type(models).__name__}" assert models, "Expected at least one model" assert len(models) <= BROWSE_LIMIT, ( f"Expected at most {BROWSE_LIMIT} models, got {len(models)}" @@ -27,9 +25,7 @@ def start(self): validate_models(models) names = [model["name"] for model in models] - assert len(names) == len(set(names)), ( - f"Expected unique model names, got {names}" - ) + assert len(names) == len(set(names)), f"Expected unique model names, got {names}" print(f"Validated {len(models)} models") @@ -37,6 +33,7 @@ def start(self): @step def end(self): + """Report successful catalog validation.""" print("BROWSE MODELS FLOW PASSED") diff --git a/flows/models/download_gguf_model_flow.py b/flows/models/download_gguf_model_flow.py index 2d10aa0..e2f92f1 100644 --- a/flows/models/download_gguf_model_flow.py +++ b/flows/models/download_gguf_model_flow.py @@ -7,7 +7,6 @@ import os from metaflow import FlowSpec, anaconda_models, step - from testdata.model_catalog_data import GGUF_MODEL @@ -15,6 +14,7 @@ class DownloadGgufModelFlow(FlowSpec): @anaconda_models @step def start(self): + """Download the configured GGUF model and validate its local artifact.""" model = self.anaconda_models.model( GGUF_MODEL["name"], format=GGUF_MODEL["format"], @@ -26,30 +26,23 @@ def start(self): f"Expected model {GGUF_MODEL['name']}, got {model.name}" ) access_denied_reason = getattr(model, "access_denied_reason", None) - assert not access_denied_reason, ( - f"Model access was denied: {access_denied_reason}" - ) + assert not access_denied_reason, f"Model access was denied: {access_denied_reason}" assert model.format == GGUF_MODEL["format"], ( f"Expected format {GGUF_MODEL['format']}, got {model.format}" ) assert model.quant_method == GGUF_MODEL["quant_method"], ( - f"Expected quantization {GGUF_MODEL['quant_method']}, " - f"got {model.quant_method}" + f"Expected quantization {GGUF_MODEL['quant_method']}, got {model.quant_method}" ) pulled_path = model.pull() - assert pulled_path == model.path, ( - f"Expected pull to return {model.path}, got {pulled_path}" - ) + assert pulled_path == model.path, f"Expected pull to return {model.path}, got {pulled_path}" assert model.download_status in ("downloaded", "skipped"), ( f"Unexpected download status: {model.download_status}" ) assert isinstance(pulled_path, str) and pulled_path.strip(), ( f"Expected a non-empty model path, got {pulled_path!r}" ) - assert os.path.isfile(pulled_path), ( - f"Downloaded model file does not exist: {pulled_path}" - ) + assert os.path.isfile(pulled_path), f"Downloaded model file does not exist: {pulled_path}" assert os.access(pulled_path, os.R_OK), ( f"Downloaded model file is not readable: {pulled_path}" ) @@ -75,6 +68,7 @@ def start(self): @step def end(self): + """Report successful GGUF model validation.""" print("GGUF MODEL DOWNLOAD FLOW PASSED") diff --git a/flows/models/download_safetensors_model_flow.py b/flows/models/download_safetensors_model_flow.py index f700334..3864334 100644 --- a/flows/models/download_safetensors_model_flow.py +++ b/flows/models/download_safetensors_model_flow.py @@ -7,7 +7,6 @@ import os from metaflow import FlowSpec, anaconda_models, step - from testdata.model_catalog_data import SAFETENSORS_MODEL @@ -15,6 +14,7 @@ class DownloadSafetensorsModelFlow(FlowSpec): @anaconda_models @step def start(self): + """Download and validate the configured Safetensors collection.""" model = self.anaconda_models.model( SAFETENSORS_MODEL["name"], format=SAFETENSORS_MODEL["format"], @@ -23,9 +23,7 @@ def start(self): assert model is not None, "Expected a model handle" access_denied_reason = getattr(model, "access_denied_reason", None) - assert not access_denied_reason, ( - f"Model access was denied: {access_denied_reason}" - ) + assert not access_denied_reason, f"Model access was denied: {access_denied_reason}" assert model.name == SAFETENSORS_MODEL["name"], ( f"Expected model {SAFETENSORS_MODEL['name']}, got {model.name}" ) @@ -39,12 +37,8 @@ def start(self): assert isinstance(model.path, str) and model.path.strip(), ( f"Expected a non-empty collection path, got {model.path!r}" ) - assert os.path.isdir(model.path), ( - f"Collection directory does not exist: {model.path}" - ) - assert os.access(model.path, os.R_OK), ( - f"Collection directory is not readable: {model.path}" - ) + assert os.path.isdir(model.path), f"Collection directory does not exist: {model.path}" + assert os.access(model.path, os.R_OK), f"Collection directory is not readable: {model.path}" assert isinstance(model.files, list) and model.files, ( "Expected a non-empty list of collection files" ) @@ -59,8 +53,7 @@ def start(self): for file_info in model.files: assert isinstance(file_info, dict), ( - f"Expected each file entry to be a dictionary, got " - f"{type(file_info).__name__}" + f"Expected each file entry to be a dictionary, got {type(file_info).__name__}" ) filename = file_info.get("filename") @@ -73,36 +66,28 @@ def start(self): f"Collection file resolves outside its directory: {filename}" ) assert os.path.isfile(file_path), f"Missing collection file: {filename}" - assert os.access(file_path, os.R_OK), ( - f"Collection file is not readable: {filename}" - ) + assert os.access(file_path, os.R_OK), f"Collection file is not readable: {filename}" actual_size = os.path.getsize(file_path) expected_size = file_info.get("size_bytes") if expected_size is not None: - assert isinstance(expected_size, int) and not isinstance( - expected_size, bool - ), ( + assert isinstance(expected_size, int) and not isinstance(expected_size, bool), ( f"Expected size_bytes for {filename} to be an integer, " f"got {type(expected_size).__name__}: {expected_size!r}" ) assert expected_size >= 0, ( - f"Expected size_bytes for {filename} to be non-negative, " - f"got {expected_size}" + f"Expected size_bytes for {filename} to be non-negative, got {expected_size}" ) if expected_size > 0: assert actual_size == expected_size, ( - f"Size mismatch for {filename}: " - f"expected {expected_size}, got {actual_size}" + f"Size mismatch for {filename}: expected {expected_size}, got {actual_size}" ) filenames.append(filename) total_size += actual_size - assert len(filenames) == len(set(filenames)), ( - f"Expected unique filenames, got {filenames}" - ) + assert len(filenames) == len(set(filenames)), f"Expected unique filenames, got {filenames}" assert total_size > 0, "Expected total collection size to be greater than zero" print(f"Downloaded collection: {model.name}") @@ -114,6 +99,7 @@ def start(self): @step def end(self): + """Report successful Safetensors collection validation.""" print("SAFETENSORS MODEL DOWNLOAD FLOW PASSED") diff --git a/flows/models/local_cache_model_flow.py b/flows/models/local_cache_model_flow.py index 6939e4c..2b57467 100644 --- a/flows/models/local_cache_model_flow.py +++ b/flows/models/local_cache_model_flow.py @@ -15,7 +15,6 @@ import uuid from metaflow import FlowSpec, anaconda_models, step - from testdata.model_catalog_data import GGUF_MODEL CACHE_ROOT = os.path.join( @@ -28,18 +27,15 @@ class LocalCacheModelFlow(FlowSpec): @anaconda_models(temp_dir_root=CACHE_ROOT) @step def start(self): + """Verify cold download and warm reuse in the task-local model cache.""" try: cold_model = self.anaconda_models.model( GGUF_MODEL["name"], format=GGUF_MODEL["format"], quant_method=GGUF_MODEL["quant_method"], ) - access_denied_reason = getattr( - cold_model, "access_denied_reason", None - ) - assert not access_denied_reason, ( - f"Model access was denied: {access_denied_reason}" - ) + access_denied_reason = getattr(cold_model, "access_denied_reason", None) + assert not access_denied_reason, f"Model access was denied: {access_denied_reason}" assert cold_model.name == GGUF_MODEL["name"], ( f"Expected model {GGUF_MODEL['name']}, got {cold_model.name}" ) @@ -47,8 +43,7 @@ def start(self): f"Expected format {GGUF_MODEL['format']}, got {cold_model.format}" ) assert cold_model.quant_method == GGUF_MODEL["quant_method"], ( - f"Expected quantization {GGUF_MODEL['quant_method']}, " - f"got {cold_model.quant_method}" + f"Expected quantization {GGUF_MODEL['quant_method']}, got {cold_model.quant_method}" ) assert not cold_model.is_collection, "Expected a single-file model" @@ -65,21 +60,16 @@ def start(self): cold_path = os.path.realpath(cold_model.path) assert cold_path == expected_path, ( - f"Model path changed after pull: expected {expected_path}, " - f"got {cold_path}" + f"Model path changed after pull: expected {expected_path}, got {cold_path}" ) assert cold_model.download_status == "downloaded", ( - f"Expected first pull to download the model, got " - f"{cold_model.download_status!r}" - ) - assert os.path.isfile(cold_path), ( - f"Downloaded model is missing: {cold_path}" + f"Expected first pull to download the model, got {cold_model.download_status!r}" ) + assert os.path.isfile(cold_path), f"Downloaded model is missing: {cold_path}" assert cold_model.files, "Cold pull did not report any model files" for file_info in cold_model.files: assert file_info["status"] == "downloaded", ( - "Expected downloaded file status 'downloaded', got " - f"{file_info['status']!r}" + f"Expected downloaded file status 'downloaded', got {file_info['status']!r}" ) cold_size = os.path.getsize(cold_path) @@ -99,32 +89,23 @@ def start(self): quant_method=GGUF_MODEL["quant_method"], ) assert warm_model is not cold_model, "Expected a new model handle" - access_denied_reason = getattr( - warm_model, "access_denied_reason", None - ) - assert not access_denied_reason, ( - f"Model access was denied: {access_denied_reason}" - ) + access_denied_reason = getattr(warm_model, "access_denied_reason", None) + assert not access_denied_reason, f"Model access was denied: {access_denied_reason}" warm_model.pull() warm_path = os.path.realpath(warm_model.path) - assert warm_path == cold_path, ( - f"Expected warm model path {cold_path}, got {warm_path}" - ) + assert warm_path == cold_path, f"Expected warm model path {cold_path}, got {warm_path}" assert warm_model.download_status == "skipped", ( - f"Expected second pull to hit the local cache, got " - f"{warm_model.download_status!r}" + f"Expected second pull to hit the local cache, got {warm_model.download_status!r}" ) assert warm_model.files, "Warm pull did not report any model files" assert len(warm_model.files) == len(cold_model.files), ( - f"Expected {len(cold_model.files)} cached files, got " - f"{len(warm_model.files)}" + f"Expected {len(cold_model.files)} cached files, got {len(warm_model.files)}" ) for file_info in warm_model.files: assert file_info["status"] == "skipped", ( - f"Expected cached file status 'skipped', got " - f"{file_info['status']!r}" + f"Expected cached file status 'skipped', got {file_info['status']!r}" ) warm_stat = os.stat(warm_path) @@ -144,6 +125,7 @@ def start(self): @step def end(self): + """Report successful task-local cache validation.""" print("LOCAL CACHE MODEL FLOW PASSED") diff --git a/flows/models/utils/model_validators.py b/flows/models/utils/model_validators.py index bda6952..470ce22 100644 --- a/flows/models/utils/model_validators.py +++ b/flows/models/utils/model_validators.py @@ -7,12 +7,10 @@ def validate_non_empty_string(value: Any, field_name: str, context: str) -> None: """Validate that a value is a non-empty string.""" assert isinstance(value, str), ( - f"{context}: field '{field_name}' must be a string; " - f"got {type(value).__name__}: {value!r}" + f"{context}: field '{field_name}' must be a string; got {type(value).__name__}: {value!r}" ) assert value.strip(), ( - f"{context}: field '{field_name}' must contain non-whitespace text; " - f"got {value!r}" + f"{context}: field '{field_name}' must contain non-whitespace text; got {value!r}" ) @@ -24,25 +22,21 @@ def validate_model(model: Mapping[str, Any], index: int) -> None: ) for field_name in ("name", "license", "source", "tags"): - assert field_name in model, ( - f"{context}: missing required field '{field_name}'" - ) + assert field_name in model, f"{context}: missing required field '{field_name}'" for field_name in ("name", "license"): validate_non_empty_string(model[field_name], field_name, context) source = model["source"] assert isinstance(source, Mapping), ( - f"{context}: field 'source' must be an object; " - f"got {type(source).__name__}: {source!r}" + f"{context}: field 'source' must be an object; got {type(source).__name__}: {source!r}" ) assert "name" in source, f"{context}: missing required field 'source.name'" validate_non_empty_string(source["name"], "source.name", context) tags = model["tags"] assert isinstance(tags, list), ( - f"{context}: field 'tags' must be a list of objects; " - f"got {type(tags).__name__}: {tags!r}" + f"{context}: field 'tags' must be a list of objects; got {type(tags).__name__}: {tags!r}" ) assert tags, f"{context}: field 'tags' must not be empty; got {tags!r}" for tag_index, tag in enumerate(tags): @@ -50,12 +44,8 @@ def validate_model(model: Mapping[str, Any], index: int) -> None: f"{context}: field 'tags[{tag_index}]' must be an object; " f"got {type(tag).__name__}: {tag!r}" ) - assert "name" in tag, ( - f"{context}: missing required field 'tags[{tag_index}].name'" - ) - validate_non_empty_string( - tag["name"], f"tags[{tag_index}].name", context - ) + assert "name" in tag, f"{context}: missing required field 'tags[{tag_index}].name'" + validate_non_empty_string(tag["name"], f"tags[{tag_index}].name", context) if "quantized_files" in model: quantized_files = model["quantized_files"] @@ -66,20 +56,16 @@ def validate_model(model: Mapping[str, Any], index: int) -> None: for file_index, file_info in enumerate(quantized_files): file_context = f"{context}, quantized file at index {file_index}" assert isinstance(file_info, Mapping), ( - f"{file_context}: expected an object, " - f"got {type(file_info).__name__}: {file_info!r}" + f"{file_context}: expected an object, got {type(file_info).__name__}: {file_info!r}" ) size_bytes = file_info.get("size_bytes") if size_bytes is not None: - assert isinstance(size_bytes, (int, float)) and not isinstance( - size_bytes, bool - ), ( + assert isinstance(size_bytes, (int, float)) and not isinstance(size_bytes, bool), ( f"{file_context}: field 'size_bytes' must be numeric, " f"not boolean; got {type(size_bytes).__name__}: {size_bytes!r}" ) assert size_bytes >= 0, ( - f"{file_context}: field 'size_bytes' must be at least zero; " - f"got {size_bytes!r}" + f"{file_context}: field 'size_bytes' must be at least zero; got {size_bytes!r}" ) diff --git a/flows/requirements-dev.txt b/flows/requirements-dev.txt new file mode 100644 index 0000000..95d1ba1 --- /dev/null +++ b/flows/requirements-dev.txt @@ -0,0 +1,6 @@ +-r requirements.txt + +pre-commit==4.6.2 +pytest==9.1.1 +pyyaml +ruff==0.16.7 \ No newline at end of file diff --git a/flows/requirements.txt b/flows/requirements.txt index e668b0f..81b4438 100644 --- a/flows/requirements.txt +++ b/flows/requirements.txt @@ -1 +1 @@ -outerbounds \ No newline at end of file +outerbounds==0.12.44 diff --git a/flows/scripts/check_flows.py b/flows/scripts/check_flows.py new file mode 100644 index 0000000..381e0ab --- /dev/null +++ b/flows/scripts/check_flows.py @@ -0,0 +1,296 @@ +"""Discover QA flows, enforce repository conventions, and run Metaflow checks.""" + +from __future__ import annotations + +import argparse +import ast +import json +import subprocess +import sys +from collections.abc import Sequence +from dataclasses import dataclass +from pathlib import Path + +FLOWS_ROOT = Path(__file__).resolve().parents[1] +REPOSITORY_ROOT = FLOWS_ROOT.parent + + +@dataclass(frozen=True) +class FlowDefinition: + path: Path + class_name: str + + +class FlowCheckError(Exception): + """Raised when repository flow validation fails.""" + + +def _is_flowspec_base(base: ast.expr, flowspec_names: set[str], metaflow_modules: set[str]) -> bool: + """Recognize FlowSpec inheritance through direct and module imports.""" + return (isinstance(base, ast.Name) and base.id in flowspec_names) or ( + isinstance(base, ast.Attribute) + and base.attr == "FlowSpec" + and isinstance(base.value, ast.Name) + and base.value.id in metaflow_modules + ) + + +def _parse_flow(path: Path) -> ast.Module: + """Parse a flow file and surface readable syntax or file errors.""" + try: + return ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + except (OSError, SyntaxError) as error: + raise FlowCheckError(f"Unable to parse {path}: {error}") from error + + +def _flow_class_names(tree: ast.Module) -> list[str]: + """Return top-level FlowSpec subclass names from a parsed flow.""" + flowspec_names: set[str] = set() + metaflow_modules: set[str] = set() + for node in tree.body: + if isinstance(node, ast.ImportFrom) and node.module == "metaflow": + flowspec_names.update( + alias.asname or "FlowSpec" + for alias in node.names + if alias.name in {"FlowSpec", "*"} + ) + elif isinstance(node, ast.Import): + metaflow_modules.update( + alias.asname or alias.name for alias in node.names if alias.name == "metaflow" + ) + + return [ + node.name + for node in tree.body + if isinstance(node, ast.ClassDef) + and any(_is_flowspec_base(base, flowspec_names, metaflow_modules) for base in node.bases) + ] + + +def validate_tracked_flow_filenames(repository_root: Path = REPOSITORY_ROOT) -> None: + """Require every tracked Python file containing a FlowSpec to use the flow suffix.""" + listed = subprocess.run( + ["git", "ls-files", "--cached", "-z", "--", "*.py"], + cwd=repository_root, + check=False, + capture_output=True, + ) + if listed.returncode: + raise FlowCheckError("Unable to list tracked flow files from the Git index") + + for raw_path in listed.stdout.split(b"\0"): + if not raw_path: + continue + relative_path = raw_path.decode("utf-8") + path = Path(relative_path) + if path.suffix != ".py" or path.name.endswith("_flow.py"): + continue + + staged = subprocess.run( + ["git", "show", f":{relative_path}"], + cwd=repository_root, + check=False, + capture_output=True, + ) + if staged.returncode: + raise FlowCheckError(f"Unable to read staged file: {relative_path}") + try: + tree = ast.parse(staged.stdout, filename=relative_path) + except SyntaxError: + continue + if _flow_class_names(tree): + raise FlowCheckError(f"Tracked FlowSpec file must end with '_flow.py': {relative_path}") + + +def _is_main_guard(test: ast.expr) -> bool: + """Recognize the standard `if __name__ == "__main__"` expression.""" + return ( + isinstance(test, ast.Compare) + and isinstance(test.left, ast.Name) + and test.left.id == "__name__" + and len(test.ops) == 1 + and isinstance(test.ops[0], ast.Eq) + and len(test.comparators) == 1 + and isinstance(test.comparators[0], ast.Constant) + and test.comparators[0].value == "__main__" + ) + + +def _has_main_guard_call(tree: ast.Module, class_name: str) -> bool: + """Return whether the main guard directly instantiates the discovered flow.""" + for node in tree.body: + if not isinstance(node, ast.If) or not _is_main_guard(node.test): + continue + return any( + isinstance(statement, ast.Expr) + and isinstance(statement.value, ast.Call) + and isinstance(statement.value.func, ast.Name) + and statement.value.func.id == class_name + for statement in node.body + ) + return False + + +def _resolve_selected_path(path: str, flows_root: Path) -> Path: + """Resolve a CLI path relative to the repository root. + + Every caller (pre-commit, CI) passes repository-relative paths. + """ + candidate = Path(path) + if candidate.is_absolute(): + return candidate.resolve() + return (flows_root.parent / candidate).resolve() + + +def discover_flows( + flows_root: Path = FLOWS_ROOT, + selected_paths: Sequence[str] = (), +) -> list[FlowDefinition]: + """Discover flows and enforce repository-wide file and naming conventions.""" + root = flows_root.resolve() + + # Validate requested paths first, but do not use them as the repository + # inventory: duplicate flow names must still be detected in unselected files. + if selected_paths: + selected = sorted({_resolve_selected_path(path, root) for path in selected_paths}) + for path in selected: + try: + path.relative_to(root) + except ValueError as error: + raise FlowCheckError(f"Flow path is outside {root}: {path}") from error + if not path.is_file(): + raise FlowCheckError(f"Flow file does not exist: {path}") + if not path.name.endswith("_flow.py"): + raise FlowCheckError(f"Flow file must end with '_flow.py': {path}") + else: + selected = [] + + # Hidden directories and Python caches can contain third-party files whose + # names happen to end in `_flow.py`; they are not repository QA flows. + paths = sorted( + path.resolve() + for path in root.glob("**/*_flow.py") + if not any( + part.startswith(".") or part == "__pycache__" for part in path.relative_to(root).parts + ) + ) + + definitions: list[FlowDefinition] = [] + for path in paths: + # One FlowSpec per file keeps discovery and per-flow CI reporting + # unambiguous. Python parsing also catches syntax errors before Metaflow. + tree = _parse_flow(path) + class_names = _flow_class_names(tree) + if len(class_names) != 1: + raise FlowCheckError( + f"{path} must define exactly one top-level FlowSpec subclass; " + f"found {len(class_names)}" + ) + class_name = class_names[0] + + # Running `python flow.py check` requires the file's main guard to + # instantiate its FlowSpec; otherwise the command can exit without checking. + if not _has_main_guard_call(tree, class_name): + raise FlowCheckError( + f'{path} must instantiate {class_name} under `if __name__ == "__main__"`' + ) + definitions.append(FlowDefinition(path=path, class_name=class_name)) + + # An empty matrix would make CI appear successful without testing anything. + if not definitions: + raise FlowCheckError(f"No *_flow.py files found under {root}") + + # Metaflow identifies flows by class name, so duplicate names across product + # domains would make runs and metadata ambiguous. + paths_by_name: dict[str, Path] = {} + for definition in definitions: + previous_path = paths_by_name.get(definition.class_name) + if previous_path is not None: + raise FlowCheckError( + f"Duplicate flow class {definition.class_name}: " + f"{previous_path} and {definition.path}" + ) + paths_by_name[definition.class_name] = definition.path + + if not selected: + return definitions + + # A selected path can pass the checks above and still be excluded from + # discovery (hidden directory, Python cache), so report it instead of + # failing with a KeyError. + definitions_by_path = {definition.path: definition for definition in definitions} + missing = [path for path in selected if path not in definitions_by_path] + if missing: + raise FlowCheckError( + "Flow file is not a discoverable repository flow: " + + ", ".join(str(path) for path in missing) + ) + return [definitions_by_path[path] for path in selected] + + +def run_metaflow_checks( + definitions: Sequence[FlowDefinition], flows_root: Path = FLOWS_ROOT +) -> None: + """Run Metaflow's built-in definition and DAG validation for every flow.""" + for definition in definitions: + relative_path = definition.path.relative_to(flows_root.resolve()) + print(f"Checking {relative_path}", file=sys.stderr) + # Native `check` owns step, transition, decorator, parameter, import, and + # graph validation. It validates definitions without executing flow steps. + result = subprocess.run( + [sys.executable, str(relative_path), "check"], + cwd=flows_root, + check=False, + ) + if result.returncode: + raise FlowCheckError( + f"Metaflow validation failed for {relative_path} with exit code {result.returncode}" + ) + + +def _parse_args(arguments: Sequence[str] | None = None) -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("paths", nargs="*", help="Flow files to validate") + parser.add_argument( + "--discovery-only", + action="store_true", + help="Discover flows without invoking Metaflow", + ) + parser.add_argument( + "--tracked-filenames-only", + action="store_true", + help="Validate tracked FlowSpec filenames from the Git index", + ) + parser.add_argument( + "--format", + choices=("paths", "json"), + default="paths", + help="Discovery output format", + ) + return parser.parse_args(arguments) + + +def main(arguments: Sequence[str] | None = None) -> int: + args = _parse_args(arguments) + flow_paths = list(args.paths) + try: + if args.tracked_filenames_only: + validate_tracked_flow_filenames() + return 0 + definitions = discover_flows(selected_paths=flow_paths) + if not args.discovery_only: + run_metaflow_checks(definitions) + except FlowCheckError as error: + print(f"Flow validation failed: {error}", file=sys.stderr) + return 1 + + paths = [definition.path.relative_to(REPOSITORY_ROOT).as_posix() for definition in definitions] + if args.format == "json": + print(json.dumps({"flow": paths})) + else: + print("\n".join(paths)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/flows/tests/test_check_flows.py b/flows/tests/test_check_flows.py new file mode 100644 index 0000000..cca9aed --- /dev/null +++ b/flows/tests/test_check_flows.py @@ -0,0 +1,246 @@ +import subprocess +import sys +from collections.abc import Sequence +from pathlib import Path + +import pytest +import scripts.check_flows as check_flows +from scripts.check_flows import ( + FlowCheckError, + FlowDefinition, + discover_flows, + run_metaflow_checks, + validate_tracked_flow_filenames, +) + + +def write_flow(path: Path, class_name: str = "ExampleFlow") -> None: + """Create the smallest flow definition needed by checker tests.""" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + "from metaflow import FlowSpec\n\n" + f"class {class_name}(FlowSpec):\n pass\n\n" + f'if __name__ == "__main__":\n {class_name}()\n', + encoding="utf-8", + ) + + +def test_discovers_flows_recursively_in_stable_order(tmp_path: Path) -> None: + """Discover nested flows deterministically while ignoring support files.""" + write_flow(tmp_path / "zeta" / "second_flow.py", "SecondFlow") + write_flow(tmp_path / "alpha" / "first_flow.py", "FirstFlow") + write_flow(tmp_path / ".venv" / "ignored_flow.py", "IgnoredFlow") + (tmp_path / "alpha" / "helper.py").write_text("VALUE = 1\n", encoding="utf-8") + + definitions = discover_flows(tmp_path) + + assert [definition.class_name for definition in definitions] == [ + "FirstFlow", + "SecondFlow", + ] + + +@pytest.mark.parametrize( + ("import_statement", "base_name"), + [ + ("from metaflow import FlowSpec as BaseFlow", "BaseFlow"), + ("import metaflow as mf", "mf.FlowSpec"), + ], +) +def test_discovers_flowspec_import_aliases( + tmp_path: Path, import_statement: str, base_name: str +) -> None: + """Recognize FlowSpec aliases in valid flow definitions.""" + path = tmp_path / "aliased_flow.py" + path.write_text( + f"{import_statement}\n\n" + f"class AliasedFlow({base_name}):\n pass\n\n" + 'if __name__ == "__main__":\n AliasedFlow()\n', + encoding="utf-8", + ) + + assert discover_flows(tmp_path) == [FlowDefinition(path.resolve(), "AliasedFlow")] + + +@pytest.mark.parametrize("destination", ["flows/renamed.py", "renamed.py"]) +def test_rejects_rewritten_flow_moved_to_filename_without_suffix( + tmp_path: Path, destination: str +) -> None: + """Reject a rewritten flow that Git cannot detect as a rename.""" + subprocess.run(["git", "init", "--quiet"], cwd=tmp_path, check=True) + original = tmp_path / "flows" / "original_flow.py" + write_flow(original, "OriginalFlow") + subprocess.run(["git", "add", "flows/original_flow.py"], cwd=tmp_path, check=True) + + original.unlink() + replacement = tmp_path / destination + replacement.parent.mkdir(parents=True, exist_ok=True) + replacement.write_text( + "from metaflow import FlowSpec as BaseFlow\n\n" + "class CompletelyRewrittenFlow(BaseFlow):\n pass\n\n" + 'if __name__ == "__main__":\n CompletelyRewrittenFlow()\n\n' + "VALUE = 1\n", + encoding="utf-8", + ) + subprocess.run(["git", "add", "--all"], cwd=tmp_path, check=True) + + with pytest.raises(FlowCheckError, match=rf"must end with '_flow.py': {destination}"): + validate_tracked_flow_filenames(tmp_path) + + +@pytest.mark.parametrize("filename", ["missing_flow.py", "helper.py"]) +def test_rejects_invalid_selected_paths(tmp_path: Path, filename: str) -> None: + """Reject missing paths and existing files without the flow suffix.""" + if filename == "helper.py": + (tmp_path / filename).write_text("VALUE = 1\n", encoding="utf-8") + + with pytest.raises(FlowCheckError): + discover_flows(tmp_path, [tmp_path / filename]) + + +def test_rejects_file_without_one_flowspec_subclass(tmp_path: Path) -> None: + """Require each discovered flow file to contain one FlowSpec subclass.""" + path = tmp_path / "invalid_flow.py" + path.write_text("class NotAFlow:\n pass\n", encoding="utf-8") + + with pytest.raises(FlowCheckError, match="exactly one"): + discover_flows(tmp_path) + + +def test_rejects_flow_without_executable_main_guard(tmp_path: Path) -> None: + """Reject a flow file that native `python flow.py check` would not execute.""" + path = tmp_path / "invalid_flow.py" + path.write_text( + "from metaflow import FlowSpec\n\nclass InvalidFlow(FlowSpec):\n pass\n", + encoding="utf-8", + ) + + with pytest.raises(FlowCheckError, match="must instantiate InvalidFlow"): + discover_flows(tmp_path) + + +def test_rejects_main_guard_that_instantiates_another_class(tmp_path: Path) -> None: + """Require the main guard to instantiate the FlowSpec declared in that file.""" + path = tmp_path / "invalid_flow.py" + path.write_text( + "from metaflow import FlowSpec\n\n" + "class InvalidFlow(FlowSpec):\n pass\n\n" + 'if __name__ == "__main__":\n OtherFlow()\n', + encoding="utf-8", + ) + + with pytest.raises(FlowCheckError, match="must instantiate InvalidFlow"): + discover_flows(tmp_path) + + +def test_rejects_empty_flow_directory(tmp_path: Path) -> None: + """Fail instead of allowing CI to produce an empty flow matrix.""" + with pytest.raises(FlowCheckError, match=r"No \*_flow.py files"): + discover_flows(tmp_path) + + +def test_reports_python_syntax_errors(tmp_path: Path) -> None: + """Surface invalid Python before invoking Metaflow validation.""" + path = tmp_path / "invalid_flow.py" + path.write_text("class InvalidFlow(FlowSpec)\n pass\n", encoding="utf-8") + + with pytest.raises(FlowCheckError, match="Unable to parse"): + discover_flows(tmp_path) + + +def test_rejects_duplicate_flow_class_names(tmp_path: Path) -> None: + """Prevent ambiguous Metaflow identities across domain directories.""" + write_flow(tmp_path / "one" / "first_flow.py", "DuplicateFlow") + write_flow(tmp_path / "two" / "second_flow.py", "DuplicateFlow") + + with pytest.raises(FlowCheckError, match="Duplicate flow class"): + discover_flows(tmp_path) + + +def test_selected_flow_still_checks_global_name_uniqueness(tmp_path: Path) -> None: + """Detect duplicate names outside the flow selected for native validation.""" + selected = tmp_path / "one" / "first_flow.py" + write_flow(selected, "DuplicateFlow") + write_flow(tmp_path / "two" / "second_flow.py", "DuplicateFlow") + + with pytest.raises(FlowCheckError, match="Duplicate flow class"): + discover_flows(tmp_path, [str(selected)]) + + +def test_selected_flow_limits_returned_definitions(tmp_path: Path) -> None: + """Return only selected flows after validating the complete repository inventory.""" + selected = tmp_path / "one" / "first_flow.py" + write_flow(selected, "FirstFlow") + write_flow(tmp_path / "two" / "second_flow.py", "SecondFlow") + + definitions = discover_flows(tmp_path, [str(selected)]) + + assert definitions == [FlowDefinition(selected.resolve(), "FirstFlow")] + + +def test_rejects_selected_path_excluded_from_discovery(tmp_path: Path) -> None: + """Report a selected path that discovery skips instead of raising KeyError.""" + write_flow(tmp_path / "one" / "first_flow.py", "FirstFlow") + hidden = tmp_path / ".venv" / "hidden_flow.py" + write_flow(hidden, "HiddenFlow") + + with pytest.raises(FlowCheckError, match="not a discoverable repository flow"): + discover_flows(tmp_path, [str(hidden)]) + + +def test_main_passes_positional_paths_through_as_selection( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Forward CLI paths to discovery unchanged; pre-commit only ever stages flow files.""" + observed_selections: list[list[str]] = [] + definition = FlowDefinition( + check_flows.FLOWS_ROOT / "models" / "browse_models_flow.py", + "BrowseModelsFlow", + ) + + def fake_discover_flows( + flows_root: Path = check_flows.FLOWS_ROOT, + selected_paths: Sequence[str] = (), + ) -> list[FlowDefinition]: + del flows_root + observed_selections.append(list(selected_paths)) + return [definition] + + monkeypatch.setattr(check_flows, "discover_flows", fake_discover_flows) + + paths = ["flows/models/browse_models_flow.py"] + assert check_flows.main(["--discovery-only", *paths]) == 0 + assert observed_selections == [paths] + + +def test_invokes_native_metaflow_check(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Delegate DAG validation to the native `python flow.py check` command.""" + path = tmp_path / "example_flow.py" + write_flow(path) + calls: list[tuple[list[str], Path]] = [] + + def fake_run(command: list[str], cwd: Path, check: bool) -> subprocess.CompletedProcess: + calls.append((command, cwd)) + return subprocess.CompletedProcess(command, 0) + + monkeypatch.setattr(subprocess, "run", fake_run) + + run_metaflow_checks([FlowDefinition(path, "ExampleFlow")], tmp_path) + + assert calls == [([sys.executable, "example_flow.py", "check"], tmp_path)] + + +def test_reports_native_metaflow_check_failure( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Turn a nonzero native Metaflow result into a checker failure.""" + path = tmp_path / "example_flow.py" + write_flow(path) + + def fake_run(command: list[str], cwd: Path, check: bool) -> subprocess.CompletedProcess: + return subprocess.CompletedProcess(command, 2) + + monkeypatch.setattr(subprocess, "run", fake_run) + + with pytest.raises(FlowCheckError, match="exit code 2"): + run_metaflow_checks([FlowDefinition(path, "ExampleFlow")], tmp_path) diff --git a/flows/tests/test_tool_versions.py b/flows/tests/test_tool_versions.py new file mode 100644 index 0000000..7e38635 --- /dev/null +++ b/flows/tests/test_tool_versions.py @@ -0,0 +1,77 @@ +import re +from pathlib import Path + +import yaml + +REPOSITORY_ROOT = Path(__file__).resolve().parents[2] +REQUIREMENTS_PATH = REPOSITORY_ROOT / "flows" / "requirements.txt" +DEV_REQUIREMENTS_PATH = REPOSITORY_ROOT / "flows" / "requirements-dev.txt" +PRE_COMMIT_CONFIG_PATH = REPOSITORY_ROOT / ".pre-commit-config.yaml" + + +def requirement_version(package: str, requirements_path: Path = DEV_REQUIREMENTS_PATH) -> str: + """Read one strictly pinned package version from a requirements file.""" + pattern = re.compile(rf"^{re.escape(package)}==([^\s#]+)", re.MULTILINE) + match = pattern.search(requirements_path.read_text(encoding="utf-8")) + # Under pre-commit only staged content is on disk, so an unstaged pin reads + # as missing here: stage the requirements and hook changes together. + assert match, f"Missing pinned {package} version in {requirements_path}" + return match.group(1) + + +def load_pre_commit_config() -> dict: + return yaml.safe_load(PRE_COMMIT_CONFIG_PATH.read_text(encoding="utf-8")) + + +def hook_config(hook_id: str) -> dict: + """Return the hook block for `hook_id` from `.pre-commit-config.yaml`.""" + for repo in load_pre_commit_config()["repos"]: + for hook in repo["hooks"]: + if hook["id"] == hook_id: + return hook + raise AssertionError(f"Missing hook {hook_id!r} in {PRE_COMMIT_CONFIG_PATH}") + + +def additional_dependency_version(hook_id: str, package: str) -> str: + """Read a pinned `package==version` entry from a hook's additional_dependencies.""" + dependencies = hook_config(hook_id).get("additional_dependencies", []) + for dependency in dependencies: + if dependency.startswith(f"{package}=="): + return dependency.split("==", 1)[1] + raise AssertionError( + f"Missing pinned {package} dependency for hook {hook_id!r} in {PRE_COMMIT_CONFIG_PATH}" + ) + + +def test_ruff_versions_match() -> None: + """Keep the Ruff CLI version aligned with the isolated pre-commit hook.""" + for repo in load_pre_commit_config()["repos"]: + if repo["repo"] == "https://github.com/astral-sh/ruff-pre-commit": + rev = repo["rev"] + break + else: + raise AssertionError(f"Missing ruff-pre-commit repo in {PRE_COMMIT_CONFIG_PATH}") + assert rev.lstrip("v") == requirement_version("ruff") + + +def test_pytest_versions_match() -> None: + """Keep the pytest CLI version aligned with its pre-commit environment.""" + assert additional_dependency_version("test-flow-tools", "pytest") == requirement_version( + "pytest" + ) + + +def test_outerbounds_versions_match() -> None: + """Validate flows against the same Outerbounds release that CI installs.""" + assert additional_dependency_version("check-flows", "outerbounds") == requirement_version( + "outerbounds", REQUIREMENTS_PATH + ) + + +def test_flow_filename_hook_matches_python_files_anywhere() -> None: + """Run staged FlowSpec filename validation outside the flows directory.""" + pattern = re.compile(hook_config("check-flow-filenames")["files"]) + + assert pattern.search("renamed.py") + assert pattern.search("flows/models/example.py") + assert not pattern.search("flows/README.md") diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..0e1243a --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,11 @@ +[tool.ruff] +target-version = "py312" +line-length = 100 + +[tool.ruff.lint] +select = ["B", "D102", "E", "F", "I", "UP"] + +[tool.pytest.ini_options] +addopts = "-q" +pythonpath = ["flows"] +testpaths = ["flows/tests"] \ No newline at end of file