fix(tests): make the image-preprocessing resize assertions actually assert - #2934
Open
Anai-Guo wants to merge 1 commit into
Open
fix(tests): make the image-preprocessing resize assertions actually assert#2934Anai-Guo wants to merge 1 commit into
Anai-Guo wants to merge 1 commit into
Conversation
…ssert The four shape checks in test_workflow_with_image_preprocessing.py are written as assert (expr, "message"), which asserts a two-element tuple. A non-empty tuple is always truthy, so these have never checked anything. Removing the parentheses alone would turn both tests red: the indices are inverted too. The workflow resizes to width=1000, height=800, and apply_resize_image calls cv2.resize(np_image, (width, height)), whose output has shape (height, width, channels) -- so shape[0] is 800 and shape[1] is 1000, while the assertions expect shape[0] == 1000 and shape[1] == 800. The messages name the right dimensions, they were just attached to the wrong axis. Fix both: drop the tuple form and match each index to the dimension its message names. Signed-off-by: Tai An <antai12232931@outlook.com>
Anai-Guo
requested review from
PawelPeczek-Roboflow,
dkosowski87,
grzegorz-roboflow,
hansent,
probicheaux,
rafel-roboflow and
yeldarby
as code owners
September 5, 2026 04:38
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
tests/workflows/integration_tests/execution/test_workflow_with_image_preprocessing.pyhas four shape checks written asassert (expr, "message"). That asserts a two-element tuple, which is always truthy — the checks have never verified anything.Both
test_resize_image_workflow_when_valid_input_providedandtest_upsize_image_workflow_when_valid_input_providedare affected, so the only thing either test actually checks about the resize is that aresized_imagekey came back.Why the parentheses were not the whole bug
Simply removing them turns both tests red — the indices are inverted as well.
RESIZE_IMAGE_WORKFLOWsetswidth: 1000, height: 800, and the block resizes withcv2.resizetakes(width, height)and returns an array shaped(height, width, channels). So the output is(800, 1000, 3):shape[0]is 800 andshape[1]is 1000, while the assertions expectshape[0] == 1000andshape[1] == 800. The messages name the right dimensions — "a width of 1000", "a height of 800" — they were just attached to the wrong axis.Reproduced standalone against the same call the block makes:
Since both tests drive the same workflow, the target dimensions are the same regardless of which fixture image goes in.
The change
One test file; each assertion drops the tuple form and is matched to the dimension its message names.
Why lint did not catch it
check_code_qualityalready selects the rule —F63coversF631 assertion is always true, perhaps remove parentheses?— butcheck_dirs := inference inference_sdk, sotests/is never linted. Running the repo's own selection against the file reports exactly these four:and after this change it reports
0.I left
check_dirsalone rather than addingteststo it: the same selection currently reports 18 findings acrosstests/(F632,F821,F824in unrelated files), so widening it is a separate piece of work. Happy to open that as a follow-up if you want it.black --checkandisort --check-onlypass on the file (it was black-clean before, and is after).Scope of verification: the shape arithmetic above was reproduced directly; I did not run the workflow integration suite itself, which needs a model manager and fixture images.
🤖 Generated with Claude Code