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

coverage: introduce ONLY_EXTRA_QUERY_PATHS #10674

Merged
merged 2 commits into from
Apr 7, 2020
Merged

coverage: introduce ONLY_EXTRA_QUERY_PATHS #10674

merged 2 commits into from
Apr 7, 2020

Conversation

junr03
Copy link
Member

@junr03 junr03 commented Apr 7, 2020

Description: introduce an environment variable that allows consuming projects to only run coverage over private extensions.
Risk Level: low
Testing: locally with Envoy Mobile as a consuming project. And existing coverage run in CI
Docs Changes: inline comment

Signed-off-by: Jose Nino jnino@lyft.com

Signed-off-by: Jose Nino <jnino@lyft.com>
test/coverage/gen_build.sh Outdated Show resolved Hide resolved
Signed-off-by: Jose Nino <jnino@lyft.com>
@junr03
Copy link
Member Author

junr03 commented Apr 7, 2020

updated @lizan

@junr03 junr03 merged commit 9931239 into envoyproxy:master Apr 7, 2020
@asraa
Copy link
Contributor

asraa commented Apr 29, 2020

This PR caused some regression when running coverage locally

test/run_envoy_bazel_coverage.sh

I believe this line shouldn't be run when $COVERAGE_TARGETS is just //test/... I hit some error here. I don't know why it doesn't get hit in CI when CI also runs with //test/...

TARGETS="$TARGETS $("${BAZEL_BIN}" query ${BAZEL_QUERY_OPTIONS} "attr('tags', 'coverage_test_lib', ${REPOSITORY}${target})" | grep "^//")"

Is that right? @junr03

Edit: disregard, sorry about that! whatever problem is happening with me running local coverage is from sometime after this, but the failure just pointed to something around there. works before the bazel 3.0 bump

@junr03
Copy link
Member Author

junr03 commented Apr 30, 2020

@asraa ok, do let me know if you find any problems!

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