[ci,bazel] Enable remote caching of FPGA & Verilator test results #25845
+36
−19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If FPGA test results can be cached through a remote cache, then we want to ensure that all nightly test runs run without this caching (for the test runs specifically, not the rest of the build), to allow us to be able to spot flaky tests when they occur. This commit achieves this by adding the `--cache_test_results=no` argument to all invocations of `./bazelisk.sh test` in the nightly Github Actions workflow. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
AlexJones0
force-pushed
the
fpga_test_remote_caching
branch
from
d635e8e to
3a9ad08
Compare
AlexJones0
changed the title
AlexJones0
force-pushed
the
fpga_test_remote_caching
branch
from
3a9ad08 to
18b1c49
Compare
Our Verilator UART and SPI DI models create pseudoterminals in order to function correctly and allow us to carry out tests in Verilator simulation environments. For these to continue functioning properly when switching to executing tests within a sandboxed environment, we must explicitly enable these options via our `.bazelrc`. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
AlexJones0
force-pushed
the
fpga_test_remote_caching
branch
from
18b1c49 to
2a8fb3e
Compare
AlexJones0
force-pushed
the
fpga_test_remote_caching
branch
from
2a8fb3e to
7cb1e16
Compare
As per https://bazel.build/reference/be/general#genrule.local, Bazel normally runs tests with a default of `local = False`. Setting `local = True` as we do for FPGA and Verilator tests by default forces genrule to use the "local" spawn strategy, which disallows remote execution, sandboxing and remote caching. We do not use remote execution, but there is no reason for our FPGA / Verilator tests to be run without sandboxing or without using remotely cached test results. Bazel will still respect the `cache_test_results` option, and so use of this cache can be overridden. Any tests that need to opt out of sandboxing features as the tests originally did can either add the `no-sandbox` tag to remove sandboxing, or the `local` tag to replicate the original functionality, though this will meant that these tests cannot be remotely cached and thus must be re-run every time if there is no result in a local cache. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
AlexJones0
force-pushed
the
fpga_test_remote_caching
branch
from
7cb1e16 to
d0a024c
Compare
nbdd0121
approved these changes
Jan 10, 2025
AlexJones0
requested review from
a-will and
jwnrt
and removed request for
rswarbrick and
cfrantz
jwnrt
approved these changes
Jan 11, 2025
pamaury
approved these changes
Jan 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In combination with the build determinism fixes from #25842, this PR aims to reduce CI runtime and FPGA runner utilisation by enabling the ability for FPGA and Verilator tests to be remotely cached. See the commit messages for more details on the individual changes.
Importantly, these changes will now mean that
Our CI bitstreams should have access to a remote cache which is written to by merge jobs, meaning that when a PR is merged, its successful FPGA/Verilator runs will be cached and can be re-used by subsequent PRs which do not modify the bitstream, testing infrastructure, or test itself. Tests that are built in some non-deterministic way will likely not be cached, however.
This PR also contains a commit to ensure that the nightly test runs do not make use of these cached results - this is to ensure that we collect FPGA test run data on a daily basis to enable us to spot e.g. flaky tests more easily, as flaky results would no longer as clearly appear in test runs on PRs.