fix: use regex to match method definitions in execution orderer - #162
Closed
avarga1 wants to merge 1 commit into
Closed
fix: use regex to match method definitions in execution orderer#162avarga1 wants to merge 1 commit into
avarga1 wants to merge 1 commit into
Conversation
Replaces str.find(f"def {name}") with a compiled regex
r"def {name}\s*\(" so that a method named e.g. "run" no longer
falsely matches a line containing "def run_all(...)".
Adds ActionStepPrefixMethods mock and test_execution_orderer_prefix_methods
to catch this regression.
Closes dotflow-io#134
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Base branch was updated from |
Member
|
Hey @avarga1! Thanks for this — really solid work, especially the test coverage with I already pushed a fix for this in PR #163 with a slightly different approach:
Since #163 is already merged, I'm going to close this one. But the test you wrote is great — would you be open to opening a separate PR just with the test ( Thanks again for the contribution! |
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
2 participants
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.
Summary
Fixes #134
_execution_ordererinexecution.pyusedstr.find(f"def {callable_name}")to locate each method in the class source. This matches any line containing that substring, so a method namedrunwould incorrectly matchdef run_all(...), causing wrong execution order or duplicate entries.Root cause (
execution.pyline 78, before fix):Fix: compile a regex that requires the name to be followed by
\s*(— i.e., only a truedef run(...)declaration matches:re.escapealso guards against method names containing regex metacharacters.Changes
dotflow/core/execution.py: importre; replacestr.findwithre.compile(...).searchtests/mocks/step_class.py: addActionStepPrefixMethods(methodsrunandrun_all)tests/mocks/__init__.py: exportActionStepPrefixMethodstests/core/test_execution.py: addtest_execution_orderer_prefix_methods— verifies each method appears exactly once andrunprecedesrun_allTest plan
tests/core/test_execution.pytests passtest_execution_orderer_prefix_methodsexplicitly catches the prefix-match regression🤖 Generated with Claude Code