feat(tests): Cover --hook-config=--tool-version resolution with pytest (#1004)

### Description of your changes

#### What
<sub>This section was generated by AI.</sub>

- Add `tests/pytest/tool_version_test.py`: black-box pytest suite for `--hook-config=--tool-version=` / `--tool-version-mode=` / `--tf-path` resolution added on `downloadable_and_version_controlled_hooks`. Tests invoke real `hooks/*.sh` scripts as subprocesses and assert only on exit code / cache filesystem state / output - never on bash-internal function names - so they need minimal changes if a hook is ever rewritten in another language.
- **Assertions prove which binary actually ran.** Each stub echoes a unique marker; every network-free test asserts that marker plus the absence of the losing candidate's. `--tool-version-mode=strict` in particular logs its NOTE *before* choosing a binary.
- Hooks now run against a coreutils-only `PATH` (no wrapped CLI tool) and an allowlisted environment (`PRE_COMMIT_COLOR=never`, `LC_ALL=C`, no inherited `PCT_TFPATH`/`TERRAGRUNT_TFPATH`/`TF_*`), so a host install or an exported variable cannot change what a test resolves. The temp repo is created with `git init --template=` and committed with `--no-verify`, because this project's own README tells users to point `init.templateDir` at a directory with pre-commit installed.
- Coverage: cache-hit reuse, `strict` vs `prefer-local`, the `--tf-path=terraform|opentofu|tofu` selector and its invalid-value error, `--tf-path` auto-detect **including Terraform winning when both binaries are on `$PATH`**, `--tf-path` literal passthrough when `--tool-version` is *not* set, the `XDG_CACHE_HOME` and `$HOME/.cache` cache roots, the documented `checkov` no-op, the actionable "tool not found" error, the `tool_name` wired by **each of the 14 resolving hooks**, and one real end-to-end download on a cache miss.
- `fix(hooks)`: resolve a pinned tool **once** per run. `common::per_dir_hook` now takes the already-resolved `tool_path` instead of re-resolving it, so hooks that need it before the per-dir loop (`tflint --init`; the terragrunt hooks choosing `hcl format` vs `hclfmt`) no longer trigger a duplicate NOTE log and, on a cache miss, a duplicate download.
- `fix(hooks)`: reject an unrecognized `--tool-version-mode` instead of treating it as `strict`, so a typo like `prefer_local` no longer silently does the opposite of what was asked. Documented in README.
- Package `hooks/`, `lib_getopt` and `tools/install/` into the sdist (`hatch.toml`), since the CI `tests` job runs pytest against a built sdist and needs the hook scripts this suite subprocesses into.
- Fix bash 3.2 (macOS default) `${var^^}` usage in `hooks/_common.sh` / `tools/install/_common.sh` (portable `tr` pipeline) and an OpenTofu release-asset version-regex bug.
- Wire `GITHUB_TOKEN` into the tox test job so the real-download test authenticates its `api.github.com` call instead of sharing the unauthenticated per-IP rate limit across the whole CI matrix.
- Mark the download test `network` (registered in `pytest.ini`, which `--strict-markers` requires) so `-m 'not network'` runs the suite offline.
- Flat test functions rather than test classes, matching this repo's existing `_cli_test.py` / `terraform_docs_replace_test.py` convention; parametrized cases are bundled into `NamedTuple`s to satisfy `PLR0913`/`WPS211` without suppressions. `.flake8` ignores `WPS226` (string-literal overuse, same precedent as `_cli_test.py`) plus `WPS202`/`WPS204`, which a wide table-driven suite trips by construction. Windows-skip coverage pragmas satisfy the `covdefaults` 100% coverage gate.
- Merge `stderr` into `stdout` in `_run_hook`, because `common::colorify` writes every diagnostic to stderr while a wrapped tool's own output goes to stdout - one ready-to-grep string per caller.

#### Why
> [!IMPORTANT]
> Somehow related/conflicts with #990. So far - I can't say what will be chosen, waiting for python-guru's review

Main idea - make some black-box testing for bash hooks on pytest, which then can be applied on hooks reimplementation on Python, when/if it time comes

Or if that's a shity approach - then just make some black-box testing, not in pytest

### How can we test changes
<sub>This section was generated by AI.</sub>

- Offline: `pytest tests/pytest/ -m 'not network'` - 43 passed, 1 deselected.
- Full suite: `pytest tests/pytest/` - 44 passed, coverage 100.00% (the `covdefaults` gate). Network test alone: `pytest -m network` - 1 passed.
- **Mutation-tested.** Each of these, applied on its own, turns the suite red: strict mode falling back to `$PATH`; inverting the `--tf-path` auto-detect precedence; dropping the `XDG_CACHE_HOME` fallback; a `terraform-docs` -> `terraform_docs` `tool_name` typo; `--tf-path` passthrough hijacked by the pinning code; `prefer-local` ignored; the `opentofu` -> `tofu` binary rename forgotten; the `--tool-version-mode` validation removed; and re-introducing the duplicate resolution inside `common::per_dir_hook`.
- Each new commit was checked out into a separate `git worktree` and its test suite run there, to confirm no intermediate commit is broken.
- This PR targets `downloadable_and_version_controlled_hooks` (not `master`) so this repo's GH Actions matrix runs the suite for real, including the network-dependent download test.
- All pre-commit hooks pass locally: `shellcheck`, `shfmt`, `ruff check`, `ruff format`, `wemake-python-styleguide`, `mypy` (py3.10/3.12/3.14).

Author: MaxymVlasov

.flake8

@@ -78,6 +78,13 @@ per-file-ignores =
     WPS226,
     # WPS115: "Require snake_case for naming class attributes" -- testing legitimate case, ignored in main code
     WPS115,
+  tests/pytest/tool_version_test.py:
+    # WPS226: "Forbid the overuse of string literals" -- same legitimate test-fixture rationale as _cli_test.py above
+    WPS226,
+    # WPS202: "Found too many module members" -- this suite covers one resolution branch per test, so the module is intentionally wide rather than deep
+    WPS202,
+    # WPS204: "Found overused expression" -- every test wires the same hermetic env/PATH sandbox and re-asserts "no download happened"; deduplicating that into helpers would hide what each test actually guarantees
+    WPS204,
   # We will not spend time on fixing complexity in deprecated hook
   src/pre_commit_terraform/terraform_docs_replace.py: WPS232
 

.github/workflows/reusable-tox.yml

@@ -311,6 +311,13 @@ jobs:
       id: tox-run
       shell: bash
       env:
+        # Consumed by tools/install/_common.sh (via `pass_env = GITHUB_*`
+        # in tox.ini) to authenticate GitHub API calls made while
+        # resolving `--tool-version` pins - without it, requests are
+        # unauthenticated and share GitHub's low per-IP rate limit
+        # across every job in this workflow's matrix, which is exactly
+        # what test_real_download_on_cache_miss can hit.
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         INPUT_TOX_RUN_POSARGS: ${{ inputs.tox-run-posargs }}
       run: |-
         tox_common_args=(

hatch.toml

@@ -2,8 +2,11 @@
 include = [
   '.codecov.yml',
   '.coveragerc',
+  'hooks/',
+  'lib_getopt',
   'src/',
   'tests/',
+  'tools/install/',
   'pytest.ini',
   'tox.ini',
 ]

hooks/_common.sh

@@ -309,14 +309,19 @@ function common::get_cpu_num {
 # 3. Complete hook execution and return exit code
 # Arguments:
 #   hook_id (string) hook ID, see `- id` for details in .pre-commit-hooks.yaml file
-#   tool_name (string) name of the wrapped tool, used to resolve its path
+#   tool_path (string) path to the wrapped tool's binary, already resolved
+#     by the calling hook via `common::resolve_tool_path`
+#     (empty for hooks with no resolvable binary, e.g. checkov).
+#     Deliberately NOT resolved here: each hook resolves once in `main`,
+#     because resolution logs a NOTE and downloads on a cache miss, so
+#     doing it again here would duplicate both
 #   args_array_length (integer) Count of arguments in args array.
 #   args (array) arguments that configure wrapped tool behavior
 #   files (array) filenames to check
 #######################################################################
 function common::per_dir_hook {
   local -r hook_id="$1"
-  local -r tool_name="$2"
+  local -r tool_path="$2"
   local -i args_array_length=$3
   shift 3
   local -a args=()
@@ -330,11 +335,6 @@ function common::per_dir_hook {
   # despite there's only one positional ARG left
   local -a -r files=("$@")
 
-  local -r tool_version=$(common::get_hook_config_value "--tool-version")
-  local tool_path
-  tool_path=$(common::resolve_tool_path "$tool_name" "$tool_version") || exit $?
-  readonly tool_path
-
   # check is (optional) function defined
   if [ "$(type -t run_hook_on_whole_repo)" == function ] &&
     # check is hook run via `pre-commit run --all`
@@ -616,6 +616,19 @@ function common::resolve_tool_path {
   #
   local -r tool_version_mode=$(common::get_hook_config_value "--tool-version-mode")
 
+  # Reject unknown values instead of silently treating them as "strict":
+  # a typo like "prefer_local" would otherwise do the exact opposite of
+  # what the user asked for, with no indication of why.
+  case "$tool_version_mode" in
+    "" | strict | prefer-local) ;;
+    *)
+      common::colorify "red" \
+        "ERROR: '--tool-version-mode=$tool_version_mode' is not a valid value.\n" \
+        "'--tool-version-mode=' must be either 'strict' (default) or 'prefer-local'."
+      exit 1
+      ;;
+  esac
+
   if command -v "$tool_name" &> /dev/null; then
     if [[ $tool_version_mode == "prefer-local" ]]; then
       common::colorify "green" \
@@ -664,7 +677,8 @@ function common::resolve_tool_path {
   common::detect_os_arch
 
   local env_var_name="${tool_name//-/_}"
-  env_var_name="${env_var_name^^}_VERSION"
+  # `${var^^}` is bash 4+ only; macOS ships bash 3.2 by default.
+  env_var_name="$(tr '[:lower:]' '[:upper:]' <<< "${env_var_name}_VERSION")"
 
   mkdir -p "$cache_dir"
 

hooks/terraform_fmt.sh

@@ -20,8 +20,13 @@ function main {
 
   local -r tool_name="tf" # Will be resolved into real tool inside 'common::resolve_tool_path'
 
+  local -r tool_version=$(common::get_hook_config_value "--tool-version")
+  local tool_path
+  tool_path=$(common::resolve_tool_path "$tool_name" "$tool_version") || exit $?
+  readonly tool_path
+
   # shellcheck disable=SC2153 # False positive
-  common::per_dir_hook "$HOOK_ID" "$tool_name" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
+  common::per_dir_hook "$HOOK_ID" "$tool_path" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
 }
 
 #######################################################################

hooks/terraform_providers_lock.sh

@@ -17,8 +17,13 @@ function main {
 
   local -r tool_name="tf" # Will be resolved into real tool inside 'common::resolve_tool_path'
 
+  local -r tool_version=$(common::get_hook_config_value "--tool-version")
+  local tool_path
+  tool_path=$(common::resolve_tool_path "$tool_name" "$tool_version") || exit $?
+  readonly tool_path
+
   # shellcheck disable=SC2153 # False positive
-  common::per_dir_hook "$HOOK_ID" "$tool_name" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
+  common::per_dir_hook "$HOOK_ID" "$tool_path" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
 }
 
 #######################################################################

hooks/terraform_tflint.sh

@@ -17,9 +17,6 @@ function main {
   # JFYI: tflint color already suppressed via PRE_COMMIT_COLOR=never
 
   local -r tool_name="tflint"
-  # Needed early for the `tflint --init` pre-flight check below, which
-  # runs once, before (and separately from) common::per_dir_hook's own
-  # resolution for the actual per-dir tflint runs.
   local -r tool_version=$(common::get_hook_config_value "--tool-version")
   local tool_path
   tool_path=$(common::resolve_tool_path "$tool_name" "$tool_version") || exit $?
@@ -39,7 +36,7 @@ function main {
     return ${exit_code}
   }
 
-  common::per_dir_hook "$HOOK_ID" "$tool_name" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
+  common::per_dir_hook "$HOOK_ID" "$tool_path" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
 }
 
 #######################################################################

hooks/terraform_tfsec.sh

@@ -23,7 +23,12 @@ function main {
 
   local -r tool_name="tfsec"
 
-  common::per_dir_hook "$HOOK_ID" "$tool_name" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
+  local -r tool_version=$(common::get_hook_config_value "--tool-version")
+  local tool_path
+  tool_path=$(common::resolve_tool_path "$tool_name" "$tool_version") || exit $?
+  readonly tool_path
+
+  common::per_dir_hook "$HOOK_ID" "$tool_path" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
 }
 
 #######################################################################

hooks/terraform_trivy.sh

@@ -15,8 +15,13 @@ function main {
 
   local -r tool_name="trivy"
 
+  local -r tool_version=$(common::get_hook_config_value "--tool-version")
+  local tool_path
+  tool_path=$(common::resolve_tool_path "$tool_name" "$tool_version") || exit $?
+  readonly tool_path
+
   # shellcheck disable=SC2153 # ARGS is set in common::parse_cmdline
-  common::per_dir_hook "$HOOK_ID" "$tool_name" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
+  common::per_dir_hook "$HOOK_ID" "$tool_path" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
 }
 
 #######################################################################

hooks/terraform_validate.sh

@@ -24,8 +24,13 @@ function main {
 
   local -r tool_name="tf" # Will be resolved into real tool inside 'common::resolve_tool_path'
 
+  local -r tool_version=$(common::get_hook_config_value "--tool-version")
+  local tool_path
+  tool_path=$(common::resolve_tool_path "$tool_name" "$tool_version") || exit $?
+  readonly tool_path
+
   # shellcheck disable=SC2153 # False positive
-  common::per_dir_hook "$HOOK_ID" "$tool_name" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
+  common::per_dir_hook "$HOOK_ID" "$tool_path" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
 }
 
 #######################################################################