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.

Sign up for GitHub

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

Jump to bottom

Magically improve coverage #6345

Merged
merged 6 commits into from
Oct 16, 2019
Merged

Magically improve coverage #6345

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cef3e6e
Delete old workaround and magically improve coverage
ryoqun Oct 14, 2019
a82a494
Restore workaround with updated comment; sadly still needed.
ryoqun Oct 16, 2019
550918c
Separate for easier debugging; also forward args
ryoqun Oct 16, 2019
98005bf
Redirect coverage stderr to file
ryoqun Oct 16, 2019
dba30e2
Compress right before uploading to save space
ryoqun Oct 16, 2019
ccafb52
Simplify
ryoqun Oct 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions scripts/coverage.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ reportName="lcov-${CI_COMMIT:0:9}"

if [[ -n $1 ]]; then
crate="--package $1"
shift
Copy link
Member Author

Choose a reason for hiding this comment

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

To forward the rest to cargo test at https://github.com/solana-labs/solana/pull/6345/files#diff-c29225768574b8edd4f3608f595c29f7R41, shift $1 here.

else
crate="--all --exclude solana-local-cluster"
fi

coverageFlags=(-Zprofile) # Enable coverage
coverageFlags+=("-Clink-dead-code") # Dead code should appear red in the report
coverageFlags+=("-Ccodegen-units=1") # Disable ThinLTO which corrupts debuginfo (see [rustc issue #45511]).
coverageFlags+=("-Ccodegen-units=1") # Disable code generation parallelism which is unsupported under -Zprofile (see [rustc issue #51705]).
coverageFlags+=("-Cinline-threshold=0") # Disable inlining, which complicates control flow.
coverageFlags+=("-Coverflow-checks=off") # Disable overflow checks, which create unnecessary branches.

Expand All @@ -37,7 +38,9 @@ rm -rf target/cov/$reportName

source ci/rust-version.sh nightly
# shellcheck disable=SC2086 #
_ cargo +$rust_nightly test --target-dir target/cov --lib $crate
RUST_LOG=solana=trace _ cargo +$rust_nightly test --target-dir target/cov --lib --no-run $crate "$@"
# shellcheck disable=SC2086 #
RUST_LOG=solana=trace _ cargo +$rust_nightly test --target-dir target/cov --lib $crate "$@" 2> /dev/null
Copy link
Member Author

@ryoqun ryoqun Oct 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Separated compile and test for easier debugging because the messages from cargo build would be also discarded altogether otherwise.

Copy link
Member

Choose a reason for hiding this comment

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

Why hide stderr here? 2> /dev/null?

Copy link
Member Author

Choose a reason for hiding this comment

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

Otherwise, tons of tons of logs will be printed on the terminal, after all we're running all test with trace level. Maybe, better off to note that as a comment?

Or, should we stash the logs to 2> $(mktemp)? I think this is overdue because we're in the coverage run. It's unlikely we need to debug using this build.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. Yes there are times when the build only fails in a coverage run in CI (we had an occurrence of this today in fact!) so being able to view the log could be helpful.

We don't need anything fancy like mktemp here though. Instead just redirect to a fixed file, coverage-stderr.log, then use https://github.com/solana-labs/solana/blob/master/ci/upload-ci-artifact.sh to include the log file as a buildkite artifact for easy access by anybody who wants it

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for explanation done 98005bf..

By the way, I'm guessing for the log file to be ranges of X00MB.. Let's see if its filesize is too big, or not... If that's concern, I'll just compress it via bz2.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yay, build passed.

Heh, I was pessimistic a bit, it seems.

@mvines Hi, are ~100MB artifacts a bit concern? Should compress?

artifact: target/cov/coverage-stderr.log
-rw-r--r-- 1 buildkite-agent buildkite-agent 94182845 Oct 16 19:03 target/cov/coverage-stderr.log
+ buildkite-agent artifact upload target/cov/coverage-stderr.log
2019-10-16 19:05:27 INFO   Found 1 files that match "target/cov/coverage-stderr.log"
2019-10-16 19:05:27 INFO   Creating (0-1)/1 artifacts
2019-10-16 19:05:28 INFO   Uploading artifact 8e17d76a-93ea-4883-99d9-3606292791b4 target/cov/coverage-stderr.log (94182845 bytes)
2019-10-16 19:05:31 INFO  Successfully annotated build

Copy link
Member

Choose a reason for hiding this comment

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

Oh wow. Sure please gzip it. I don't expect this to be used very often so the extra decompression step for the user should be no big deal

Copy link
Member Author

Choose a reason for hiding this comment

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

it really shrank!

-rw-r--r-- 1 buildkite-agent buildkite-agent 10056926 Oct 16 23:51 target/cov/coverage-stderr.log.gz
+ buildkite-agent artifact upload target/cov/coverage-stderr.log.gz
2019-10-16 23:52:52 INFO   Found 1 files that match "target/cov/coverage-stderr.log.gz"
2019-10-16 23:52:52 INFO   Creating (0-1)/1 artifacts
2019-10-16 23:52:52 INFO   Uploading artifact 98e02c81-9b3f-456d-95ee-4826660c7cb3 target/cov/coverage-stderr.log.gz (10056926 bytes)
2019-10-16 23:52:54 INFO  Successfully annotated build


echo "--- grcov"

Expand Down