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

Should driver rebuild non-test targets? #37

Open
luxe opened this issue Dec 26, 2022 · 1 comment
Open

Should driver rebuild non-test targets? #37

luxe opened this issue Dec 26, 2022 · 1 comment

Comments

@luxe
Copy link

luxe commented Dec 26, 2022

The driver tool works great for running tests that were affected by changes. However, targets that are not dependencies of any test target, will be skipped. If we were to gate using this driver, we would not catch compile errors on non-test targets.

Should the driver be extended to also build non-test targets? Non-test targets are correctly identified by target_determinator but appear skipped by the driver. For our CI, I'm thinking of using target_determinator to get all the targets, then writing our own driver that does both build / test (not just test).

The commandVerb does start out as build , but it seems like a single test target will turn the command to test? I wonder if it would be better to split the targets by build / test targets. Or in my case, I might try to run all the targets as build then all the targets as test.

Thoughts?

@illicitonion
Copy link
Collaborator

Non-test targets are correctly identified by target_determinator but appear skipped by the driver.

They should end up being built; if you bazel test //some:binary //some:test, bazel should build both targets, but only run the second test - your build should fail if //some:binary fails to build. Do you have an example you can share where this isn't the case?

I wonder if it would be better to split the targets by build / test targets.

In general I wouldn't recommend that, as you'll lose parallelism - some of the build and test could otherwise be done in parallel, and now will be forced to be serialised.

For our CI, I'm thinking of using target_determinator to get all the targets, then writing our own driver that does both build / test (not just test).

target-determinator was very intentionally written as a building block to make it easy to integrate into your own custom driver, because in my experience most organisations have subtly different desires from their workflows - honestly the bundled driver was meant more as an example of how to do this rather than something most people would pick up off the shelf, but in the hopes that it's sufficient for many smaller organisations' use-cases without needing further customisation. If it's broken in some way (and I would count "doesn't fail in the case of build failures" as broken), I'd be very happy to write or review a fix, but it's also intended that folks can build their own more complex logic around it if they prefer :)

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