Do not emit a build problem when there are no tests to run (exit code 4)

[odisseus]

In a build chain with multiple stages, a test stage sometimes has no tests to run. This can happen if the target pattern file is populated by a Bazel query at the previous stage.

In such cases, Bazel emits a special exit code which has a very specific meaning and doesn't occur in any other situations. In my opinion, this exit code should not be treated as a build problem.

I know at least one company which uses a shell script to swallow these spurious build problems:

bazel test --target_pattern_file=affected_targets
EXIT=$?
[ $EXIT == 4 ] && echo "No targets to test" || exit $EXIT

Unfortunately, this precludes them from using the Bazel plugin.

See also https://github.com/bazelbuild/bazel/issues/7291#issuecomment-1283970438.

[JayBazuzi]

I generally consider "no tests found" to be a fatal error. Treating that as success creates a loophole where we believe we are protected by our tests but in fact they are not running. I like that Bazel treats this as an error condition.

However, I don't understand the specific situation you describe. Can you explain what you mean by a multi-stage build and why it can't use the build verb instead?

[odisseus]

One possible use-case would be using bazel-diff to determine the set of targets that need to be rebuilt and re-tested. It can happen that the list of affected targets doesn't contain any tests, and feeding that list as an input to a testing step would trigger the false build problem.

P.S. I agree that this behaviour can be dangerous in some cases. For example, an error in build configuration may cause the test set to be always empty, even though the user has intended otherwise.

[odisseus]

Is there any conceptual difference between exit code 4 and exit code 3 (build OK, but some tests failed)? For what it's worth, the latter also gets a special treatment in the current version of this plugin (1, 2, commit).

[JohnnyMorganz]

Should this then be explicitly opted into when creating the bazel runner step? It seems bad to slip this through as successful if the intention was to actually test something, like mentioned above. If we explicit select "don't fail when no test targets found", then intentions are clear.

[JayBazuzi]

To carry that thinking further: that should be an option in Bazel, not something introduced by this plugin.

[JohnnyMorganz]

that should be an option in Bazel

yup, i think that's what the comment is pointing to in the OP. No movement since 2022 though :(

[odisseus]

Even if they add this functionality in the latest version of Bazel, upgrading a project to use it can require a great deal of work, especially if there are dependencies on third-party rules. Upgrading the TeamCity plugin is actually easier in some cases.

[vmayushan]

I am not sure yet we can accept this as it will be breaking change, we might have to implement a way to control it. I will discuss it with the team. Could you again explain a bit the use case when tests filter can lead to zero tests? How can it happen?

[odisseus]

Here's an example provided by my co-worker.

Suppose we have a simple Bazel project with a library, a binary that depends on it, and a test for the library:

cc_library(foo) -> cc_binary(bar)
               \-> cc_test(foo_test)

We set a build in which we first run bazel-diff (we can use the example script). The Kotlin definition of the build would look like this:

steps {
    script {
        name = "Determine affected targets"
        scriptContent = "bazel-diff-example.sh > affected_targets"
    }

    bazel {
        name = "Test affected targets"
        command = "test"
        arguments = "--target_pattern_file=affected_targets"
    }
}

• If a dev makes a change on foo, affected_targets will contain foo, bar and foo_test. Running bazel test --target_pattern_file=affected_targets results in foo and bar being built and foo_test being tested. All good.
• If a dev makes a change on foo_test, affected_targets will contain foo_test. Running bazel test --target_pattern_file=affected_targets results in foo being built and foo_test being tested. All good.
• If a dev makes a change on bar, affected_targets will contain bar. Running bazel test --target_pattern_file=affected_targets results in foo and bar being built, but no tests will be run. Bazel will exit with code 4.

We could mitigate this by checking the output of bazel-diff, but this would incur extra queries. Moreover, we would need to switch between bazel build and bazel test depending on this condition, which would result in a very complicated build.

[vmayushan]

Hi again, I am sorry for delay, had some urgent tasks.

I have talked to the teammates and we think we can use value of already existing checkbox "one of build steps exited with an error (e.g non-zero exit code in command line runner)" in "Failure conditions"

I will check if it's possible this value from the runner and will update you here

[odisseus]

I'm afraid this feature is not a good substitute, because the failure conditions can only be configured for the whole build. In a workflow with multiple steps, the exit code may be the only signal of failure for some steps.

In the example I provided above, bazel-diff can exit with a non-zero code (e.g. due to a failed query). If that happens, we'd want to abort the build before the Bazel tests are even started.

[odisseus]

Here's how the setting looks in my custom-built version of the plugin:

[odisseus]

I have added a condition to show the new setting only if the command of the current build step is "test". However, the effect (showing or hiding the field) occurs only when the build step settings are saved and reopened.

P.S. Fixed in commit d52e71433d56346d530e50090daf3f87714cfc1f.

[odisseus]

Are there any remaining problems with this pull request?

[vmayushan]

Hi! Everything looks good to me, let's merge it! Thank you and sorry for the merge delay