integration-tests: tweaks to stabilize sanitizer runs #357
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
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
oschaaf
force-pushed
the
ci-sanitizer-run-stabilizations
branch
from
14954cc to
0d24955
Compare
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
|
I filed #356 for tracking the thresholds getting exceeded. If that is fixable, then we could revert some of the expectations I updated here. |
1 task
mum4k
approved these changes
Jun 15, 2020
mum4k
pushed a commit
that referenced
this pull request
Jun 19, 2020
Following some digging into stability of our coverage testing, creating this PR for testing CI with some fixes / enhancements, as well as covering a few more lines in our CLI tooling exception paths. There seems to be something racy going on where sometimes the coverage originating from our python integration tests is not counted. Eventually I learned that things might stabilise when we switch the toolchain to clang 10. We follow Envoy's lead on that, so for now stopping further efforts on this. ### Description of the changes in this PR: - Increase timeouts of ASAN/TSAN runs - Make `envoy_build_sha.sh`, which apparently broke in one of the Envoy updates - Cover some of the outer exception handling paths for Nighthawk's CLIs - Integration test for nighthawk_output_transform - Add coverage for code lines where it seemed trivial to do so (e.g. NullStatistic) - Fix `run_nighthawk_bazel_coverage.sh` to work on my machine - Stricter bash options in some scripts + changes to make that work ### Prerequisites - [x] Needs #357 to go in first. --------- Leaving some notes/learnings here: - there seems to be something racy going on where sometimes the coverage originating from our python integration tests is not counted. This is relatively infrequent in CI. - On my dev machine this structurally happens. This might be a toolchain/version issue. - On my dev machine, using the docker flow, I have difficulties: the docker container exits when running the full test suite, without leaving a trace why. bash traps in the container don't fire, no useful logging from docker. Eventually I was able to get past bazel fetching its deps, and then build just the python integration tests. As long as a single test is involved, things seem to work / the container gets to finish. It's remarkable that CI doesn't have this problem. Coverage structurally looks as expected in this scenario. - The link step is super memory hungry. Separating that step might be a ci perf opportunity. Coverage is the slowest CI job we have.
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.
This PR has changes to deflake integration tests for sanitizer runs.
for them in edge cases. Update expectations to match that.
Fixes #332