-
Notifications
You must be signed in to change notification settings - Fork 2.2k
refactor: get_sources_to_compile
#11464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
4fc020c
to
996cea7
Compare
cmd.args(["test"]).with_no_redact().assert_success().stdout_eq(str![[r#" | ||
... | ||
Compiling 21 files with [..] | ||
Compiling 23 files with [..] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the extra 2 files are script/Counter.s.sol and forge-std/Script.sol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the idea for get_sources_to_compile
was that we don't want to compile e.g scripts for forge test
, is this no longer the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My hunch was that in practice it ends up being slower to compile another time before to filter out unneeded files than to just run the pipeline normally; I can revisit further by rewriting to solar
* refactor: test filter * refactor: handling of empty test filters * dontfail * fix
get_sources_to_compile
currently is always called, even when there are no filters to apply to the input. This is wasteful because it will compile all sources to ABI, filter out nothing, then recompile everything again for bytecode regardlessrun_tests
to simplify the function and so we can also handle the case when there are no tests at all in the project