Skip to content

Fix: run the update-constraints pre-commit hook in check mode - #163

Open
AmaadMartin wants to merge 4 commits into
mainfrom
fix/update-constraints-hook-check-mode
Open

Fix: run the update-constraints pre-commit hook in check mode#163
AmaadMartin wants to merge 4 commits into
mainfrom
fix/update-constraints-hook-check-mode

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):

Prior art: google#6505 proposes the same
one-line change, bundled with four other edits that have since landed
separately.

  1. Or, if no issue exists, describe the change:

Problem: The Pre-commit Linter job fails on every commit to main, while
the other 15 jobs pass. The update-constraints hook runs
./scripts/update_constraints.sh with no flag, which is the script's update
mode. Update mode re-derives --exclude-newer from the current date, rewrites
all five constraints-*.txt headers, and exits non-zero whenever it rewrote a
file. An auto-fixer that exits 1 on success cannot pass as a gate.

Solution: Pass --check in the hook entry. The script's own usage block
already designates --check as the CI mode. Check mode reads the recorded
--exclude-newer date back out of each file header, so it is deterministic and
never writes to a tracked file. It still fails on real pin drift, so the guard
is kept, not deleted.

Two new tests pin the contract: the hook entry must pass --check, and every
Python version in the script's PYTHON_VERSIONS array must have a committed
constraints file.

Deliberately not re-shipped, because they already landed on main: the
astral-sh/setup-uv step in the lint job (566fca3f), the five
constraints-*.txt files and the codespell skip (42f220a6). This PR touches
one YAML file and adds one test module.

Collision check against the open fork PRs. Three PRs edit the same six lines,
and none of them use --check:

Five more PRs (#54, #68, #77, #149, #159) edit
scripts/update_constraints.sh. This PR does not touch that script, so it does
not conflict with them.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.
pytest tests/unittests/scripts/test_update_constraints_hook_config.py -q
2 passed

Both tests were proved able to fail, by mutating the exact line each one pins:

Mutation Failure
Remove --check from the hook entry AssertionError: The update-constraints hook must pass --check ... / assert '--check' in []
Delete constraints-3.14.txt AssertionError: No committed constraints file for Python ['3.14'] ...
Delete .pre-commit-config.yaml FileNotFoundError on the config read

Manual End-to-End (E2E) Tests:

Run from the repo root. Needs uv and network access to PyPI.

uvx pre-commit run update-constraints --all-files
update-constraints.......................................................Passed
git status --porcelain   # empty: the hook rewrote nothing

Both failure paths were exercised the same way:

Injected fault Result
Downgrade the a2a-sdk pin in constraints-3.12.txt Failed, exit 1, constraints-3.12.txt is OUT OF DATE! plus the remedy line. The file was left as-is, so check mode wrote nothing.
Delete constraints-3.14.txt Failed, exit 1, constraints-3.14.txt is missing!

All other hooks pass on the changed files:

uvx pre-commit run --files tests/unittests/scripts/test_update_constraints_hook_config.py .pre-commit-config.yaml
check yaml / fix end of files / trim trailing whitespace / isort / pyink /
addlicense / ADK Compliance Checks / codespell ....................... Passed

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

Amaad Martin added 3 commits August 7, 2026 16:57
Update mode re-derives --exclude-newer from the current date and exits
non-zero whenever it rewrites a file, so the hook could never pass in the
Pre-commit Linter job. Check mode reads the recorded date back out of each
constraints file header, writes nothing, and still fails on real pin drift.

Adds two guard tests: the hook entry must pass --check, and every Python
version in the script's PYTHON_VERSIONS array must have a committed
constraints file.
The hand-rolled walk-up returned Path | None, which forced two
assert _REPO_ROOT is not None narrowing lines that the module-level
skipif already makes unreachable. Match the sibling test file and index
parents[3] instead, then drop both asserts.
…ests

The skipif predicate can never be true. tests/ is never packaged (the sdist
includes only src, README, pyproject and LICENSE), so .pre-commit-config.yaml
is present wherever these tests run. The guard also inverted the intent it was
meant to serve: a deleted .pre-commit-config.yaml is the most severe hook-config
drift there is, and the skip turned that into a silent pass.

Both tests still fail on their mutations, and deleting .pre-commit-config.yaml
now fails instead of skipping.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant