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

Building and running tests in a single command #695

Open
cbarrete opened this issue Jun 20, 2024 · 5 comments
Open

Building and running tests in a single command #695

cbarrete opened this issue Jun 20, 2024 · 5 comments

Comments

@cbarrete
Copy link
Contributor

This is a continuation of facebookincubator/buck2-change-detector#3 (comment).

The idea is to have a way of building all the targets and running all the tests that match a target pattern in a single invocation of buck2. This would be an improvement in terms of efficiency and convenience when testing changes, either locally or in CI. This is required because buck2 test does not build the targets that are not depended on by tests (e.g. binary targets that are not integration tested, or untested code).

@ndmitchell suggested adding a --build flag to test. I have started looking into implementing it and have two remarks:

  • I would appreciate pointers on how to best do it as I'm not yet familiar with the codebase.
  • It seems to me that a --test flag to buck2 build might make more sense for a couple of reasons:
    • buck2 build will have already built all the test targets, so we would "only" need to run them. On the other hand, doing the opposite means teaching buck2 test to build more targets. Of course, this is just my intuition, I don't know yet if it is actually simpler in implementation.
    • There are fewer options to buck2 test, so it would be easier to support them from buck2 build versus the other way around.

I am willing to do the work given a few hints, but if someone more familiar with the codebase wants to pick it up because it is quick enough, feel free to!

@cbarrete
Copy link
Contributor Author

I have created #702 to add the flag, I'd appreciate any feedback/review.

Note that builds are not filtered by --exclude at the moment. I would like to implement that as a follow up, but my understanding is that DefaultInfo does not have a first class concept of labels like ExternalRunnerTestInfo does. Is that accurate, and if so, what would be the best way to get those labels to perform the filtering?

@zjturner
Copy link
Contributor

How about a bxl?

@cbarrete
Copy link
Contributor Author

BXL is not user friendly (at least until #86 is addressed IMO), especially if it is shoved somewhere deep in the prelude.

Teaching buck2 build, buck2 test and buck2 run is simple and makes sense, and adding a flag is also straightforward enough. Telling people "use those first class commands day to day, and by the way use something completely different and advanced when you want to both build and test" is not a good experience to me.

Moreover, I would like the path of least resistance to be as close to what runs in CI (which is typically the right/canonical/source of truth way of building a project). Of course, CI has target determination and a few other things, but being able to run a single, first-class command that both builds and tests a project (ideally in all relevant configurations, but that's a separate issue) means that what devs run locally is closer to CI by default.

My experience is that many people default to using what is simpler: raw cmake/ctest vs the build script/command that sets up the right environment first, cargo test vs the xtask that does things "better", and I expect: buck2 test vs some BXL script that requires more typing and isn't well understood by everyone.

@zjturner
Copy link
Contributor

zjturner commented Jun 25, 2024

Buck2 can invoke itself recursively, so you can have buck2 build invoke buck2 bxl.

You could also write a wrapper for buck2 that parses the first argument and then runs buck2 bxl if it’s test.

That said, i think in general you’re trying to do too much with too few tools, there are a lot of things that simply won’t work unless you reach outside the vanilla buck2+meta prelude experience.

I agree buck2 test should work, but that’s a long ways off and it’s not viable to block yourself on it

@cbarrete
Copy link
Contributor Author

I'm not blocked, I have scripts the build then test and it's fine for now. I could turn that into a BXL script if I wanted to.

What I'm suggesting is at least a nice QOL improvement that fits in a +41/-26 commit. I'm not saying that the buck2 command should fulfill all of my needs out of the box, but we can also have nice things by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants