-
Notifications
You must be signed in to change notification settings - Fork 100
Add Code Coverage GH action #654
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
base: main
Are you sure you want to change the base?
Conversation
@brunoocasali, I ran this action in my fork. Make sure |
WalkthroughA new GitHub Actions workflow file, Changes
Assessment against linked issues
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/coverage.yml (4)
3-11
: Simplify triggers by filtering branches at theon
level
You can move branch restrictions into theon
block to avoid complexif
logic and unnecessary job invocations:on: - pull_request: - push: - branches: - - trying - - staging - - main + pull_request: + branches: [trying, staging, main] + push: + branches: [trying, staging, main]This makes it clear which branches trigger the workflow and lets you remove some of the conditional complexity below.
28-33
: Consider using the officialactions-rs/toolchain
action
Instead of the community forkdtolnay/rust-toolchain@nightly
, the maintainedactions-rs/toolchain@v1
is more widely used and regularly updated:- - uses: dtolnay/rust-toolchain@nightly - with: - components: llvm-tools-preview + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: nightly + override: true + components: llvm-tools-preview
39-45
: Fail fast on coverage steps
Wrap the combined commands with a stricter shell mode to stop on any error and avoid proceeding with incomplete data:- - name: Collect coverage data - run: | + - name: Collect coverage data + run: | + set -euo pipefail cargo llvm-cov --no-report --all-features --workspace cargo llvm-cov --no-report --doc --all-features --workspace cargo llvm-cov report --doctests --codecov --output-path codecov.jsonThis will immediately break if any subcommand fails.
46-50
: Pin the Codecov Action to a specific patch version
Using@v5
can introduce unexpected changes when the action is updated. Pin to a known release, e.g.:- - name: Upload coverage reports to Codecov - uses: codecov/codecov-action@v5 + - name: Upload coverage reports to Codecov + uses: codecov/codecov-action@v5.1.2This locks you to the tested version and avoids surprises on minor bumps.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
.github/workflows/coverage.yml
(1 hunks)
🔇 Additional comments (1)
.github/workflows/coverage.yml (1)
12-15
: Review required permissions
The Codecov Action may need additional permissions to post status checks or PR comments. Currently you have:permissions: contents: read issues: writeConsider narrowing and adding only what’s required, for example:
permissions: contents: read checks: write pull-requests: writePlease verify the minimum scopes the Codecov Action requires.
- name: Meilisearch (latest version) setup with Docker | ||
run: docker run -d -p 7700:7700 getmeili/meilisearch:latest meilisearch --no-analytics --master-key=masterKey |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add a health check to ensure Meilisearch is ready before coverage runs
Right now the container is started but there's no wait, which can lead to flaky failures if the service isn't up when tests run. For example:
- - name: Meilisearch (latest version) setup with Docker
- run: docker run -d -p 7700:7700 getmeili/meilisearch:latest meilisearch --no-analytics --master-key=masterKey
+ - name: Meilisearch (latest version) setup with Docker
+ run: |
+ docker run -d --name meilisearch -p 7700:7700 getmeili/meilisearch:latest meilisearch --no-analytics --master-key=masterKey
+ # Wait until Meilisearch is healthy
+ until curl --silent --fail http://localhost:7700/health; do
+ sleep 1
+ done
This ensures the service is reachable before coverage collection begins.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- name: Meilisearch (latest version) setup with Docker | |
run: docker run -d -p 7700:7700 getmeili/meilisearch:latest meilisearch --no-analytics --master-key=masterKey | |
- name: Meilisearch (latest version) setup with Docker | |
run: | | |
docker run -d --name meilisearch -p 7700:7700 getmeili/meilisearch:latest meilisearch --no-analytics --master-key=masterKey | |
# Wait until Meilisearch is healthy | |
until curl --silent --fail http://localhost:7700/health; do | |
sleep 1 | |
done |
🤖 Prompt for AI Agents
In .github/workflows/coverage.yml at lines 37 to 38, the Meilisearch Docker
container is started without a health check, which can cause flaky test failures
if the service isn't ready. Add a step after starting the container that waits
for Meilisearch to be reachable, such as using a loop with curl or a similar
command to poll the service endpoint until it responds successfully before
proceeding with coverage collection.
# Will not run if the event is a PR to bump-meilisearch-v* (so a pre-release PR) | ||
# Will still run for each push to bump-meilisearch-v* | ||
# Will not run if the actor is Dependabot (dependabot PRs) | ||
if: github.event_name != 'pull_request' || !startsWith(github.base_ref, 'bump-meilisearch-v') || github.actor != 'dependabot[bot]' | ||
runs-on: ubuntu-latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix the conditional logic to properly skip bump and Dependabot PRs
The current if
uses OR (||
) in a way that still runs the job for bump-meilisearch PRs and Dependabot PRs, contrary to the intended behavior. To skip when it's a PR and either the base ref starts with bump-meilisearch-v
or the actor is Dependabot, update to:
- if: github.event_name != 'pull_request' || !startsWith(github.base_ref, 'bump-meilisearch-v') || github.actor != 'dependabot[bot]'
+ if: github.event_name != 'pull_request' || ( !startsWith(github.base_ref, 'bump-meilisearch-v') && github.actor != 'dependabot[bot]' )
This ensures only non-PR events or PRs targeting non-bump branches by non-Dependabot actors will run.
🤖 Prompt for AI Agents
In .github/workflows/coverage.yml around lines 21 to 25, the conditional logic
in the `if` statement incorrectly uses OR operators, causing the job to run for
bump-meilisearch PRs and Dependabot PRs when it should skip them. Update the
condition to use AND and OR properly so that the job only runs if the event is
not a pull request or if it is a pull request but the base ref does not start
with 'bump-meilisearch-v' and the actor is not 'dependabot[bot]'. This will
correctly skip the job for bump and Dependabot PRs.
Pull Request
Related issue
Fixes #507
What does this PR do?
This pull request introduces a new GitHub Actions workflow for measuring and reporting code coverage. The workflow is configured to run on specific branches and includes steps to set up the environment, install necessary tools, and upload coverage reports to Codecov.
Key changes:
coverage.yml
to measure code coverage ontrying
,staging
, andmain
branches.cargo-llvm-cov
for coverage data collection.PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit