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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code coverage tooling #448

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

svix-dylan
Copy link
Contributor

No description provided.

server/run-tests-with-grcov.sh Outdated Show resolved Hide resolved
server/run-tests-with-grcov.sh Show resolved Hide resolved
Comment on lines +4 to +14
bash -c "
# tell Rust to run with coverage instrumentation
RUSTFLAGS=\"-Cinstrument-coverage\"
# give grcov a profile name template for output files
LLVM_PROFILE_FILE=\"svix-webhooks-%p-%m.profraw\"
# put the compiler in nightly mode
RUSTC_BOOTSTRAP=1

# run tests
./run-tests.sh
" || true
Copy link
Member

Choose a reason for hiding this comment

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

Why are you doing this in a subscript like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So that these variables are only set in this new session, and don't affect rustc in any subsequent commands a developer might run unrelated to this script.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you do this:

RUSTFLAGS="-Cinstrument-coverage" \
LLVM_PROFILE_FILE="svix-webhooks-%p-%m.profraw" \
RUSTC_BOOTSTRAP=1 \
./run-tests.sh

...

Copy link
Member

Choose a reason for hiding this comment

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

You don't need to do anything, it won't affect anything outside of the script.
Also, I just noticed you didn't run export so actually these won't even work at all.

@@ -0,0 +1,19 @@
#!/bin/sh -e
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the -e needed here?

Comment on lines +4 to +14
bash -c "
# tell Rust to run with coverage instrumentation
RUSTFLAGS=\"-Cinstrument-coverage\"
# give grcov a profile name template for output files
LLVM_PROFILE_FILE=\"svix-webhooks-%p-%m.profraw\"
# put the compiler in nightly mode
RUSTC_BOOTSTRAP=1

# run tests
./run-tests.sh
" || true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
bash -c "
# tell Rust to run with coverage instrumentation
RUSTFLAGS=\"-Cinstrument-coverage\"
# give grcov a profile name template for output files
LLVM_PROFILE_FILE=\"svix-webhooks-%p-%m.profraw\"
# put the compiler in nightly mode
RUSTC_BOOTSTRAP=1
# run tests
./run-tests.sh
" || true
# tell Rust to run with coverage instrumentation
export RUSTFLAGS="-Cinstrument-coverage"
# give grcov a profile name template for output files
export LLVM_PROFILE_FILE="svix-webhooks-%p-%m.profraw"
export TEST_NIGHTLY="+nightly"
# run tests
./run-tests.sh

And then in run-tests:

TEST_COMMAND="cargo ${TEST_NIGHTLY} test --all --all-features --all-targets -- --test-threads=1"

Copy link
Member

@tasn tasn May 20, 2022

Choose a reason for hiding this comment

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

Or better yet, just tell people to use nightly (see other comment), and then:

#!/bin/sh -e

# tell Rust to run with coverage instrumentation
export RUSTFLAGS="-Cinstrument-coverage"

# give grcov a profile name template for output files
export DIRNAME="./target/debug/coverage/$(date -u --iso-8601=seconds)"
mkdir -p "$DIRNAME"
export LLVM_PROFILE_FILE="$DIRNAME/svix-webhooks-%p-%m.profraw"

echo "Collecting coverage in $DIRNAME"

# run tests
./run-tests.sh

# generate and open report output
echo "Generating coverage report (may take time)."
~/.cargo/bin/grcov "$DIRNAME" -s svix-server --binary-path ./target/debug/ -t html --branch --ignore-not-existing -o "$DIRNAME"

echo "Coverage HTML file at:\n$(pwd)/$DIRNAME/index.html"

Copy link
Member

@tasn tasn left a comment

Choose a reason for hiding this comment

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

Let's not worry about it for now, just dumped my thoughts.

# Code Coverage
To get a code coverage report, first make sure you have the following installations:
- `cargo install grcov`
- `rustup component add llvm-tools-preview`
Copy link
Member

Choose a reason for hiding this comment

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

This won't work if default is not set to nightly. Probably just need to say "use nightly" which will also simplify the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran both of these install commands on stable successfully

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, they install fine, but I had issues. Though maybe my issues were unrelated.

Comment on lines +4 to +14
bash -c "
# tell Rust to run with coverage instrumentation
RUSTFLAGS=\"-Cinstrument-coverage\"
# give grcov a profile name template for output files
LLVM_PROFILE_FILE=\"svix-webhooks-%p-%m.profraw\"
# put the compiler in nightly mode
RUSTC_BOOTSTRAP=1

# run tests
./run-tests.sh
" || true
Copy link
Member

@tasn tasn May 20, 2022

Choose a reason for hiding this comment

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

Or better yet, just tell people to use nightly (see other comment), and then:

#!/bin/sh -e

# tell Rust to run with coverage instrumentation
export RUSTFLAGS="-Cinstrument-coverage"

# give grcov a profile name template for output files
export DIRNAME="./target/debug/coverage/$(date -u --iso-8601=seconds)"
mkdir -p "$DIRNAME"
export LLVM_PROFILE_FILE="$DIRNAME/svix-webhooks-%p-%m.profraw"

echo "Collecting coverage in $DIRNAME"

# run tests
./run-tests.sh

# generate and open report output
echo "Generating coverage report (may take time)."
~/.cargo/bin/grcov "$DIRNAME" -s svix-server --binary-path ./target/debug/ -t html --branch --ignore-not-existing -o "$DIRNAME"

echo "Coverage HTML file at:\n$(pwd)/$DIRNAME/index.html"

@svix-dylan
Copy link
Contributor Author

svix-dylan commented May 20, 2022

Adding some thoughts here. Apologies about some confusion earlier in this PR, I was playing with multiple strategies and artifacts from both were left in the commits.

grcov uses one of two strategies to generate code coverage: source-based or "gcov"-based coverage (presumably via the gcc utility). Per further research, source-based coverage supports (at least) proc macros, and generally seems to be the preferred approach. This PR has only ever implemented source-based coverage (despite the presence of RUSTC_BOOTSTRAP=1).

Agreed, leaving this for now.

@tasn tasn linked an issue May 23, 2022 that may be closed by this pull request
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.

Add code coverage to CI (+ a badge)
3 participants