bazel: avoid invalidating analysis cache between build and tests.#7294
bazel: avoid invalidating analysis cache between build and tests.#7294lizan merged 2 commits intoenvoyproxy:masterfrom
Conversation
This is achieved by pulling BAZEL_TEST_OPTIONS into BAZEL_BUILD_OPTIONS, i.e. using the same build options for "build" and "test" commands. Signed-off-by: Piotr Sikora <piotrsikora@google.com>
5e86aaa to
5dc9678
Compare
|
This doesn't have any measurable effect on the CI, but I think this is still a good change. cc @lizan |
|
|
||
| # Test options | ||
| test --test_env=HEAPCHECK=normal --test_env=PPROF_PATH | ||
| build --test_env=HEAPCHECK=normal --test_env=PPROF_PATH |
There was a problem hiding this comment.
Hmm, I don't think this change is needed?
There was a problem hiding this comment.
It is, otherwise --test_env changes between bazel build and bazel test.
There was a problem hiding this comment.
right, but that doesn't affect cache analysis.
There was a problem hiding this comment.
It does:
$ bazel --noworkspace_rc build //source/common/buffer:buffer_lib 2>&1 | grep test_env
$ bazel --noworkspace_rc test --test_env=TEST=1 //source/common/buffer:buffer_lib 2>&1 | grep test_env
INFO: Build option --test_env has changed, discarding analysis cache.
$ bazel --noworkspace_rc build --test_env=TEST=1 //source/common/buffer:buffer_lib 2>&1 | grep test_env
$ bazel --noworkspace_rc test --test_env=TEST=1 //source/common/buffer:buffer_lib 2>&1 | grep test_env
There was a problem hiding this comment.
With .bazelrc from master (which sets different --test_env flags for bazel test):
$ bazel build //source/common/buffer:buffer_lib 2>&1 | grep test_env
$ bazel test //source/common/buffer:buffer_lib 2>&1 | grep test_env
INFO: Build option --test_env has changed, discarding analysis cache.
After the change from this PR:
$ bazel build //source/common/buffer:buffer_lib 2>&1 | grep test_env
$ bazel test //source/common/buffer:buffer_lib 2>&1 | grep test_env
There was a problem hiding this comment.
Hmm, it seems bazel discard analysis cache while action cache is still valid so the build doesn't take longer time. If you feel strong about this change go for it. Can you merge master in that case?
There was a problem hiding this comment.
I think it makes sense to add it, otherwise this PR feels half-done. Merged master. Thanks!
…eedup Signed-off-by: Piotr Sikora <piotrsikora@google.com>
we should do memory test exact with docker runs too. |
The exact memory test is tied to a given libstdc++ version, which means that it might fail, depending on the Docker container you're running, whereas But I don't feel too strongly about it, just pointing it out. |
This is achieved by pulling BAZEL_TEST_OPTIONS into BAZEL_BUILD_OPTIONS,
i.e. using the same build options for "build" and "test" commands.
Signed-off-by: Piotr Sikora piotrsikora@google.com