facebook / buck2

Build system, successor to Buck
https://buck2.build/
Apache License 2.0
3.51k stars 214 forks source link

Building and running tests in a single command #695

Open cbarrete opened 3 months ago

cbarrete commented 3 months ago

This is a continuation of https://github.com/facebookincubator/buck2-change-detector/issues/3#issuecomment-1979723079.

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 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 commented 3 months ago

I have created https://github.com/facebook/buck2/pull/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 commented 3 months ago

How about a bxl?

cbarrete commented 2 months ago

BXL is not user friendly (at least until https://github.com/facebook/buck2/issues/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 commented 2 months ago

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 commented 2 months ago

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.