Convert test runs into actions

[moroten]

I have written a draft about how Bazel can let rule authors depend on test results, for example to aggregate and generate test reports during the build phase. It basically creates a TestResultInfo and exposes ctx.actions.run_test.

The document is available at https://docs.google.com/document/d/1PMRwGzpF1SR7JGHK6OvzlRJeL2Z5oXS6xUpq6ts9Ffc/edit?usp=sharing

This is just an initial draft. I will fill in more details if the proposal sounds reasonable and something the Bazel community is happy to adopt.

Best regards,
Fredrik

[justinhorvitz]

I believe that non-tests depending on tests has caused issues with --trim_test_configuration. @sdtwigg do you have an opinion on this?

Separately, it's also caused issues with our build cleaner.

[fmeum]

We could require rules depending on tests to be marked testonly, which should help with test configuration trimming (see --experimental_retain_test_configuration_across_testonly).

[moroten]

Satus so far:

1. Generating test reports inside Bazel is a step towards turning Bazel into a general purpose workflow management tool, which it very specifically does not want to be. Otherwise, TestResultInfo could be exposed from all test rules.
2. Adding execution requirement ignore-exit-code makes it possible to run a test in a genrule and then follow up with an sh_test. The UI and BES will be fooled about what is the actual test execution, but that's just cosmetic. Flaky tests won't be supported this way.
3. Test identifiers in BES do not support aspects adding even more tests. Can be solved by saying aspects are adding extra shards, backwards compatible with all tools.
4. testonly=True is defined on target level, does not match if aspects are allowed to add tests on non-testonly targets.

Point 1 is a blocker for the whole concept. I believe that test report generation inside Bazel is valid as I would like to bundle them with my release package, which I would like to be produced by Bazel.

Point 2 feels like a workaround, but will solve some of my problems.

I think point 3 can be solved in a backward compatible way.

At the moment, I don't know how to solve point 4.

[lberki]

We met with @moroten in person and BazelCon and discussed this proposal a bit.

It would be a big change to how Bazel works (for example, one would suddenly be able to run actions that depends on failed actions, which didn't use to be the case, aspects would be able to generate test actions which the BEP does not support, etc.) and it also has a large number of "unknown unknowns", at least to me -- I can't say with confidence what invariants this proposal would break inside Bazel and in systems that depend on it.

It's not an absolute contraindication, but it requires a commensurately large benefit to the community and from the lack of enthusiastic support on this thread and the bazel-discuss thread, it looks like there isn't a widespread need.

So I propose to shelve this idea until such support materializes; if it does exist, I could imagine implementing a prototype that's not checked in to discover what issues would arise within Bazel, then if it works well, submitting it behind a --experimental_* flag, etc., in short, the usual lifecycle of a new feature. It would be a long road and volunteers to do the coding would probably be needed, though.

[sluongng]

For what is worth, a handful of our customers have expressed difficulty switching some of their internal projects to Bazel because it’s hard to integrate with external systems that track code coverage overtime such as SonarCloud or continuous profiling systems.

Having this feature would help rules owners support language-specific coverage format and test profiling data a lot easier.

If the current API is too disruptive, let’s find a way to ship a more restrictive API that could be extended incrementally overtime? I am down to have them guarded behind —exp_ flag(s) if that make it easier, though do keep in mind that with the current release cadence, it will take anywhere from 2-12 months before these flags reached end users.

[sluongng]

Perhaps, to slow roll this out, we would want to first create a Starlark version of the current TestRunner action without any additional feature added on top.

Something like

my_test = rule(
  impl = build_impl,
  test_impl = test_impl,
  …
)

where test_impl will replace the current is_test attribute. It shall come with a very restrictive test_context that only provides access to build_impl returned providers to construct a single test action? a default test_impl could be provided in skylib for backward compatible reasons. 🤔

[moroten]

testonly (is that what you meant) can be set on a standard cc_library, e.g. helper libraries m meant for test code, not production code.

[sluongng]

https://bazel.build/extending/rules#executable_rules_and_test_rules i meant replacing "test = True" in "rule" function with an actual starlark lambda that defines the TestRunner action.

[moroten]

Okay, I'll have to think through what implications I can see there.

[moroten]

One conclusion is that we should not implement anything that only a minority of the community wants, the maintenance cost becomes too high then.

If you were interested in this idea, please vote for it.