Add a --build flag to buck2 test

[cbarrete]

No description provided.

[cbarrete]

@ndmitchell @JakobDegen Hello, would you mind having a look when you have a moment?
Thanks a lot!

[JakobDegen]

Hm, interesting, I assumed we were already doing this. Tbh I'm half of the mind that this should be the default behavior and we shouldn't even support the current thing, but I have no idea what the history on this choice is and if others agree with me. I'll make a post in our internal discussion group to try and a few more eyes on this issue (will direct discussion over here of course)

[cbarrete]

Thanks, I look forward to hearing from this!

I don't know about the history either, but I can see the current behavior making sense as well, as a target pattern which matches expensive targets (e.g. binaries that pull in a lot of targets and/or are heavily templated; packages/release tarballs/OCI images/etc.) would cause a "simple" buck2 test to be surprisingly expensive, but I personally wouldn't mind making this the default (potentially with a flag to opt out/only build what is necessary for tests).

[JakobDegen]

One additional thought: If we do go the direction of a flag, we should probably use the same --build-{default,run}-info flags that buck2 build uses, instead of coming up with something new. But let's agree on whether we should have a flag at all before getting into that.

Also, @cbarrete , can you say a little bit about your use case for this?

[cbarrete]

I just used --build because that's what Neil suggested first, likely without thinking too much about it. I'm happy to rename it to whatever fits best, change the details of how things get wired, etc. I'll leave the bikeshedding to people who actually have a clear vision for the CLI!

Also, @cbarrete , can you say a little bit about your use case for this?

Ultimately, I'm trying to simplify/improve our CI setup. Right now we have a script that looks vaguely like:

targets=$(supertd stuff)
buck2 build --skip-incompatible-targets $targets
buck2 build --skip-incompatible-targets $targets --target-platforms //non/default:platform
buck2 build --skip-incompatible-targets $targets --target-platforms //some/other:platform

buck2 test --skip-incompatible-targets $targets
buck2 test --skip-incompatible-targets $targets --target-platforms //non/default:platform
buck2 test --skip-incompatible-targets $targets --target-platforms //some/other:platform

Which I'd ideally like to turn into:

targets=$(supertd stuff)
buck2 test $targets --configurations //default:platform //non/default:platform //some/other:platform

I'm omitting test runner business, but I want something that lets me send individual test logs to a backend for a nicer UX than everything in one file. I'll also gloss over the configuration stuff, that's a separate issue (in short, it would be nice to not run them sequentially, but also some targets in different language that aren't affected by the target incompatibility end up being tested multiple times, which is suboptimal).

So what's relevant here is merging the build and test steps together, which is both nicer and more efficient (fewer DAGs = more goodness overall).

The end goal is to make CI so vanilla that what devs run locally is as close to what runs remotely as possible by default. The differences would be the supertd business, which is ok to me, and something people can opt into; and the custom test runner, which is not relevant locally. I have a personal opinion that it would be a GoodThing for CI systems to be very thin and leverage good build systems as much as possible, so that plays into that as well.

[JakobDegen]

Alright, since people internally seem to have surprisingly few opinions on this, I say let's land it without changing the default for now. That being said, I would ask you to switch away from a --build flag and instead offer both --build-run-info and --build-default-info, matching what buck2 build has

[cbarrete]

Hey, I haven't forgotten about that, but just haven't had the time to finish this up. I hope to get to it before the end of the week though.

[facebook-github-bot]

@JakobDegen has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[cbarrete]

Hello @JakobDegen, is something blocking this diff internally? Is there something I can do to help move this forward?