Skip to content
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

feat: add --skip <filter> to forge build #3370

Merged
merged 5 commits into from
Sep 26, 2022

Conversation

mattsse
Copy link
Member

@mattsse mattsse commented Sep 26, 2022

Motivation

Closes #3366

add --skip <filter> to exclude matching contracts, with aliases for tests (--skip tests) and scripts (--skip scripts).

--skip tests scripts for excluding *.[s].sol contracts

cc @tynes

Solution

@mattsse mattsse added T-feature Type: feature Cmd-forge-build Command: forge build labels Sep 26, 2022
@mattsse mattsse marked this pull request as ready for review September 26, 2022 21:11
@mattsse mattsse merged commit 226affb into foundry-rs:master Sep 26, 2022
prj.write_config(config);

// only builds the single template contract `src/*`
cmd.args(["build", "--skip", "tests", "--skip", "scripts"]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, but should we change to test and skip to match the default folder names? or was this intentional to not match those folder names?

Suggested change
cmd.args(["build", "--skip", "tests", "--skip", "scripts"]);
cmd.args(["build", "--skip", "test", "--skip", "script"]);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that actually makes more sense, I guess we can just do test[s]? and script[s]?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems reasonable to me!

@tynes
Copy link
Contributor

tynes commented Sep 28, 2022

Thank you!

@holic
Copy link

holic commented Sep 30, 2022

This is great! Thoughts on adding this to forge test and/or foundry config (for applying to both commands)?

Currently, if I run forge build --skip test script, I get just the files I need in out. Then if you run forge test, build files from tests end up appearing in out.

(I can work around this temporarily by specifying a separate --out for tests)

@mds1
Copy link
Collaborator

mds1 commented Sep 30, 2022

I'd prefer not adding --skip to forge test for two reasons:

@holic Why do you want your test artifacts in a different directory than contract artifacts?

@holic
Copy link

holic commented Sep 30, 2022

It's not that I want to skip particular tests, but rather skip generating build files for tests.

I am checking into git the contract artifacts (the results of forge build created in out), specifically ABIs, but don't need to check in test/script artifacts (e.g. SomeTest.t.sol).

I use these ABIs to generate types (also checked in), and including tests and scripts means there's a bunch of unnecessary type generation happening. In the type generation step, I am currently ignoring these files manually: https://github.com/holic/web3-scaffold/blob/main/packages/contracts/package.json#L14

If I instead --skip these as part of forge build, I don't need to do any ignoring like above, but I still get tests artifacts being generated when running forge test (as in the forge output is no longer the same between build and test).

Lemme know if all that makes sense?

@holic
Copy link

holic commented Sep 30, 2022

Writing all that out, I am now thinking it might be better for me to have a specific build command for generating checked-in files (e.g. ABIs) to their own directory rather and trying to conditionally ignore certain build files in out (and gitignore the whole build/out dir).

iFrostizz pushed a commit to iFrostizz/foundry that referenced this pull request Nov 9, 2022
* feat: add --skip <filter> to forge build

* feat: add skip filter

* integrate filter

* chore: bump ethers

* test: pin version

Co-authored-by: Georgios Konstantopoulos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmd-forge-build Command: forge build T-feature Type: feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

forge build --skip-tests
5 participants