Issue #8664 | Bug 🐛: --match-path does not work with watch mode: cannot be used multiple times #8664 #8709
Conversation
This file contains 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
…mode: cannot be used multiple times foundry-rs#8664
PabloVillaplana
requested review from
DaniPopes,
Evalir and
mattsse
as code owners
klkvr
reviewed
Aug 21, 2024
crates/forge/bin/cmd/watch.rs
Outdated
|
|
||
| // Check if any of the filters are present | ||
| let path_pattern_exists = filter.args().path_pattern.is_some(); | ||
|
|
||
| // Marker to check whether to override the command. | ||
| let _no_reconfigure = filter.args().test_pattern.is_some() || |
There was a problem hiding this comment.
I think we should just use no_reconfigure here, looks like this was used earlier to avoid updating the command if any filters were set
There was a problem hiding this comment.
Done ✅, thank you for the review 👍🏾
There was a problem hiding this comment.
please remove newlines and I think there's no need for underscore in var name name
klkvr
requested changes
Aug 21, 2024
crates/forge/bin/cmd/watch.rs
Outdated
|
|
||
| // Check if any of the filters are present | ||
| let path_pattern_exists = filter.args().path_pattern.is_some(); | ||
|
|
||
| // Marker to check whether to override the command. | ||
| let _no_reconfigure = filter.args().test_pattern.is_some() || |
There was a problem hiding this comment.
please remove newlines and I think there's no need for underscore in var name name
klkvr
approved these changes
Aug 21, 2024
benwjhack
pushed a commit
to CompassLabs/foundry-test
that referenced
this pull request
Sep 11, 2024
…mode: cannot be used multiple times foundry-rs#8664 (foundry-rs#8709) * Issue foundry-rs#8664 | Bug 🐛: --match-path does not work with watch mode: cannot be used multiple times foundry-rs#8664 * reuse _no_reconfigure var * Remove undescore var name and also remove no needed extra lines * fmt --------- Co-authored-by: Arsenii Kulikov <[email protected]>
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.
Issue Link -> #8664
Pull Request Description
Currently the following command was being attempted to be executed:
forge test --match-path test/Counter.t.sol -w
But by default it can be done without the --math-path, since the command adds it by itself
--match-path does not work with watch mode: cannot be used multiple times
`error: the argument '--match-path ' cannot be used multiple times
Usage: forge test [OPTIONS]
For more information, try '--help'.
[Command exited with 2, lasted 10.7375ms]`
Motivation
My motivation is to continue contributing to tools like Foundry, to learn from them and how they work in Open Source environments. I really like to always be in constant practice and what better way to do it than by practicing and helping the community.
Solution
Currently when running the command:
forge test --match-path test/Counter.t.sol --watch
It is not necessary to add the ---match-path because by default the --watch is added by code, now what I did was add a validation that if it already has it in the arguments, do not add it to avoid redundancy and errors like the reported issue