Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

circleci: upload artifacts when coverage is below threshold #2502

Merged
merged 2 commits into from
Feb 1, 2018

Conversation

lizan
Copy link
Member

@lizan lizan commented Feb 1, 2018

Signed-off-by: Lizan Zhou [email protected]

Description:
Run rsync before checking threshold so CircleCI can pick up the coverage report.

Risk Level: Low
Testing: local tested
Docs Changes: N/A
Release Notes: N/A

COVERAGE_VALUE=$(grep -Po 'lines: \K(\d|\.)*' "${COVERAGE_SUMMARY}")
COVERAGE_THRESHOLD=98.0
COVERAGE_THRESHOLD=99.0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will revert this to 98.0 once I confirmed this actually works.

Signed-off-by: Lizan Zhou <[email protected]>
@lizan
Copy link
Member Author

lizan commented Feb 1, 2018

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @lizan. @htuch for bash check.

@@ -73,6 +73,8 @@ time "${GCOVR}" --gcov-exclude="${GCOVR_EXCLUDE_REGEX}" \
# run.
rm "${SRCDIR}"/test/coverage/BUILD

[[ -z "${ENVOY_COVERAGE_DIR}" ]] || rsync -av "${COVERAGE_DIR}"/ "${ENVOY_COVERAGE_DIR}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only if the coverage dir doesn't exist? I often (locally) use bazel.coverage to generate reports, and will run it a number of times to validate the coverage status as I make code changes. Usually it just clobbers the previous report as currently implemented. In this PR, I think it would silently fail to update.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-z just check if the env ENVOY_COVERAGE_DIR is set, not checking if dir exists, if you run test/run_envoy_bazel_coverage.sh directly, this env var is not set.

@htuch htuch merged commit 6dbcbbb into envoyproxy:master Feb 1, 2018
Shikugawa pushed a commit to Shikugawa/envoy that referenced this pull request Mar 28, 2020
…2502)

* use destination service name in stackdriver metric label

* add todo

* fix merge mistake
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants