search

filmil / bazel-bats

bazel-bats: bazel test rules for the BATS testing framework (based on bats-core)
Apache License 2.0
10 stars 4 forks source link

Bazel rule succeeds when it should be failing #3

Closed noel-yap closed 2 years ago

noel-yap commented 3 years ago

I'm seeing the following output:

$ bazel test
...
//ops/releng/scripts:releng_test                                         PASSED in 2.0s
$ for b in *.bats; do echo $b; bats $b; done
...
rollback-pods.bats
 ✓ should error on too few arguments
 ✓ should error on too many arguments
 ✓ happy path
 ✓ happy path with absolute revision
   happy path with relative revision                                                                                                        5/5
   bats warning: Executed 4 instead of expected 5 tests

What can cause something like this?

noel-yap commented 3 years ago

It happens when there's at least one bats file that passes.

noel-yap commented 3 years ago

I found the culprit. Some of my tests have a trap to remove temporary files. If I comment out the trap, bazel fails as expected.

I'm still unable to fix this, though, since trap 'es=$?; rm -rf -- "${tmpdir}"; exit ${es}' INT TERM HUP EXIT isn't causing bazel to fail.

filmil commented 3 years ago

IIUC, the root issue here is that you create a temporary directory for your tests. It then interacts badly with bazel's test result detection.

This should not be necessary in bazel: when running the tests, bazel will give you an env variable TEST_TMPDIR that you can use for a hermetic writeable area. I understand that this can be a confusing situation when you want to use both bazel and "vanilla" test invocations.

See: https://docs.bazel.build/versions/main/test-encyclopedia.html#initial-conditions for details.

Within a test it may make sense to check whether you are running under bazel, and then use the provided temporary directory instead of managing your own.

filmil commented 2 years ago

I'm assuming that this result came out of using the test fixture in a way that was not intended. Closing for now, but feel free to reopen if you disagree.