villebro commented on PR #44250:
URL: https://github.com/apache/superset/pull/44250#issuecomment-5751188320
Picking up the three additional suggestions from Bito run #9b6078, all in
`tests/unit_tests/scripts/docker_cache_ref_test.py`. Addressed in 5a30e8f995.
**Suffix allowlist drift risk** — the real one. `PRESET_SUFFIXES` hard-coded
`-dev`/`-lean`, but `scripts/docker-build-extra-flags.sh` appends
`-${BUILD_PRESET}` from a generic case arm, so adding a matrix preset there
would have left the allowlist quietly stale. Rather than couple the test to the
script's preset list, it now matches the suffix generically
(`(?:-[a-z0-9]+)?`): the invariant worth enforcing is the base Python version,
not which presets happen to exist. Verified it still accepts `-dev`/`-lean` and
a hypothetical `-py313`, still rejects a wrong base version
(`3.12-slim-bookworm`), and rejects a malformed double suffix.
**Missing helper and test docstrings** — added. Worth noting for future runs
that the cited `BITO.md` rules (12147, 12148, 12490) are not discoverable in
this repo; there is no `BITO.md` here, so those IDs only resolve inside the
Bito workspace. The changes stand on `CLAUDE.md`'s own "Docstrings: Required
for new functions/classes" instead. The local convention in that directory is
also narrower than the cited rule — `change_detector_test.py` gives docstrings
to helpers and to tests whose rationale is non-obvious, while leaving
self-describing test names bare — so please read a bare test name there as
house style rather than an omission.
Re-verified after the change: 5 passed, and the negative check (temporarily
bumping `ARG PY_VER` in the Dockerfile) still fails all five consumers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]