fix(step): don't crash when a single foreach reference resolves falsy - #6726
Closed
Prabal864 wants to merge 1 commit into
Closed
fix(step): don't crash when a single foreach reference resolves falsy#6726Prabal864 wants to merge 1 commit into
Prabal864 wants to merge 1 commit into
Conversation
Step._get_foreach_items used the `X and Y or Z` idiom to pick between returning a single resolved foreach value and zip()-ing multiple &&-combined ones together. That idiom is only safe when Y is never falsy: a foreach reference that legitimately resolves to 0, False, or "" fell through to zip(*foreach_items) despite there being only one item, and crashed with TypeError since scalars aren't iterable. Replaced it with an explicit conditional that branches on len(foreach_items) == 1 directly instead of on the truthiness of the resolved value. Closes keephq#6721
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6726 +/- ##
===========================================
- Coverage 46.42% 30.46% -15.96%
===========================================
Files 178 101 -77
Lines 18694 11773 -6921
===========================================
- Hits 8679 3587 -5092
+ Misses 10015 8186 -1829 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Closing this in favor of finding and fixing a fresh, not-yet-reported issue instead of one that was already filed. |
5 tasks
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's changed?
Step._get_foreach_itemsused the classicX and Y or Zidiom to decide whether to return a single resolvedforeachvalue directly, orzip()multiple&&-combined ones together:That idiom is only safe when
foreach_items[0]is never falsy. If aforeach: "{{ ... }}"reference legitimately resolves to a falsy scalar (0,False,""), the expression falls through tozip(*foreach_items)even though there's only one item — andzip()requires an iterable argument, so it crashes withTypeErrorinstead of returning the resolved value.Fixed by replacing the idiom with an explicit conditional that branches on
len(foreach_items) == 1directly, rather than on the truthiness of the resolved value:No behavior changes for the happy path (truthy single value, or multiple
&&-combined references) — only the previously-crashing falsy-single-value case is fixed.Closes #6721
How was this patch tested?
Added
tests/test_step_foreach_falsy_value.py, which constructs a realStep(with a realContextManager) and calls the actual_get_foreach_items, covering:foreachreference resolving to0,False,"", or0.0now returns that value instead of raisingTypeError&&-combined references stillzip()together correctlyI wasn't able to run this test file (or the full suite) in my sandbox — same as the previous PR (#6720):
poetry installfails on this platform becauseuvloop(a pyproject dependency) doesn't support Windows, so the project's actual dependencies never fully install. What I did verify:python -m py_compilepasses on both changed files.black --checkandisort --checkpass on both changed files._get_foreach_items's return statement, verbatim) into a standalone script and ran it directly against0,False,"", a truthy list, multiple&&-combined references, and the empty case: the old code raisesTypeErroron0/False(and silently returns[]instead of""for the empty-string case, viazip(*[""])iterating zero characters) while the new code returns the correct value in every case, with no change for the already-working cases.A maintainer running this on Linux/macOS (where
uvloopinstalls fine) should seetests/test_step_foreach_falsy_value.pypass outright.Checklist