testing: include offending values in validation error messages - #79
Conversation
kstonekuan
left a comment
There was a problem hiding this comment.
Thank you @ayam04, and welcome! You took the optional stretch sites as well as the two required ones, and the messages read the way the rest of the codebase does.
What I validated locally:
- Full quality gate is clean (
ruff check,ruff format --check,ty check) and all 307 tests pass. - Re-ran the gate on your branch merged with current
main, since #77 landed a change totesting.pya few minutes ago. It merges cleanly and stays green, so no rebase needed on your side. - Confirmed
nameis in scope at thesteps.pysite (it is the first parameter ofcompute_check_version) and that no test asserts the old message strings.
On the tests/test_ffmpeg.py failures you saw: those are environment, not your change. Those tests need real ffmpeg/ffprobe binaries on PATH, and they pass here. Thank you for checking against a clean worktree before reporting it; that is exactly the right instinct, and it saved me from wondering.
Nice detail: keeping camera_name on !r so "" and " " read differently in the message. That was the whole point of that one.
Merging now. The open good first issues have plenty left if you want another (issues with an assignee are taken; everything else is fair game), and we hang out on Discord.
What changed
Four validation errors now include the offending value(s), matching the pattern already used by every other raise in
_validate_video_episode_spec(e.g.source_start_s must be >= 0, got ...):src/hflow/testing.py—camera_name must not be empty, got {spec.camera_name!r}(the check is.strip()-based, so""vs" "is exactly the distinction the message should surface)src/hflow/testing.py—black_segment and freeze_segment must not overlap, got {spec.black_segment!r} and {spec.freeze_segment!r}src/hflow/steps.py—declared step version must not be empty for step {name!r}(the optional stretch named in the issue;nameis in scope)src/hflow/app.py—derived channel topic must not be empty, got {topic!r}(optional stretch)No test asserts these strings (per the issue), so nothing else needed updating.
Validation
tests/test_ffmpeg.pyshows3 failed, 6 errorsboth on this branch and on cleanmain(verified in a detached worktree) — pre-existing WSL/environment ffmpeg-binary detection issues, unaffected by this change.Fixes #65