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

Change set -x in coverage to be set by var #13248

Conversation

keith
Copy link
Member

@keith keith commented Mar 19, 2021

Using set -x in the coverage scripts causes a lot of noise, this changes it to be off by default, but something you can enable by passing --test_env=VERBOSE_COVERAGE=1 for debugging.

@keith keith requested a review from lberki as a code owner March 19, 2021 22:03
@google-cla google-cla bot added the cla: yes label Mar 19, 2021
@lberki lberki requested review from c-mita and removed request for lberki March 22, 2021 07:43
@finn-ball
Copy link
Contributor

These scripts are quite broken in general. I found it very useful to leave these flags in when debugging what was going wrong with coverage.

Out of curiosity, I see you also removed the -x flag from the collect_cc_coverage.sh - do you happen to use this with remote execution?

@brentleyjones
Copy link
Contributor

We plan on using code coverage with remote execution, yes. For context, we are doing Swift code coverage, not C++.

@keith
Copy link
Member Author

keith commented Mar 22, 2021

I definitely get the debugging use case, it's too bad we don't have an easy way to have both here. Maybe we can conditionally set -x based on some --test_env like --test_env=DEBUG_COVERAGE=1

This causes logs to be very noisy with coverage infrastructure logic
@keith keith force-pushed the ks/remove-set-x-from-coverage-collection branch from d583f14 to 64013e1 Compare March 22, 2021 21:42
@keith keith changed the title Remove set -x from coverage collection Change set -x in coverage to be set by var Mar 22, 2021
@keith
Copy link
Member Author

keith commented Apr 16, 2021

@c-mita can you review?

@bazel-io bazel-io closed this in 615e1b1 Apr 20, 2021
@keith
Copy link
Member Author

keith commented Apr 20, 2021

Thanks!

philwo pushed a commit that referenced this pull request Apr 20, 2021
Using `set -x` in the coverage scripts causes a lot of noise, this changes it to be off by default, but something you can enable by passing `--test_env=VERBOSE_COVERAGE=1` for debugging.

Closes #13248.

PiperOrigin-RevId: 369396233
@keith keith deleted the ks/remove-set-x-from-coverage-collection branch June 22, 2021 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants