Repository navigation
Combined unit/integration test code coverage in CI - #289
alan-george-lk wants to merge 1 commit into
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| - name: Run integration tests | ||
| timeout-minutes: ${{ inputs.integration_timeout_minutes }} |
There was a problem hiding this comment.
🟡 Disabled integration tests still run in coverage
When integration_repeat is zero, coverage still runs the integration suite. The matrix job honors that input, so unit-only invocations unexpectedly run server-dependent tests.
Learn more
The reusable workflow accepts integration_repeat as the switch for integration testing. The matrix job gates server setup, CLI installation, token server setup, and the integration step on values above zero in the test job. The coverage job now performs these actions even when that switch is zero. A caller can request coverage while disabling integration, but the coverage job still exercises integration tests and fails if its server-dependent setup fails.
Example: A manual invocation sets run_coverage=true and integration_repeat=0. The matrix jobs skip their integration steps, while the coverage job starts both servers and runs the integration binary.
Recommended fix: Gate all four new setup and execution steps in coverage on inputs.integration_repeat > 0, preserving the unit-only coverage path when integration is disabled.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - name: Run integration tests | ||
| timeout-minutes: ${{ inputs.integration_timeout_minutes }} | ||
| env: | ||
| RUST_LOG: "metrics=debug" | ||
| LIVEKIT_CREATE_TOKEN_URL: ${{ steps.token_server.outputs.token-url }} | ||
| run: | | ||
| set -euo pipefail | ||
| source scripts/set-test-tokens.sh | ||
| # TODO(BOT-477) Temporarily disable platform audio integration tests in CI until instability is resolved | ||
| bash scripts/run-with-backtrace.sh \ | ||
| build-debug/bin/livekit_integration_tests \ | ||
| --gtest_filter='-PlatformAudioIntegrationTest.*' \ | ||
| --gtest_output=xml:build-debug/integration-test-results.xml |
There was a problem hiding this comment.
🟡 Integration failures erase coverage results
When livekit_integration_tests fails, coverage skips report generation despite completed unit tests. The HTML artifact and summary disappear instead of showing the available coverage.
Learn more
The coverage job runs unit tests, then integration tests, then Generate coverage reports and Add coverage summary to job. GitHub Actions skips steps with an implicit success() condition after a failed step. The upload step runs on failure, but there is no HTML report to upload if gcovr was skipped. This changes an integration-only failure into loss of otherwise available unit coverage output.
Example: Unit tests pass and write .gcda; an integration assertion fails. The job fails, gcovr never runs, and the always-run artifact step has no coverage HTML to upload.
Recommended fix: Run gcovr and the summary under an appropriate if: success() || failure() condition after tests, with guards for missing build outputs; preserve the failure status of the integration step.
Was this helpful? React with 👍 or 👎 to provide feedback.
stephen-derosa
left a comment
stephen-derosa
left a comment
There was a problem hiding this comment.
how does GTest know what % of integration is covered? For unit its a bit easier to understand "do all lines of code get hit" (give or take), but how does it work for integration?
Also looks like Devin left some reasonable comments
| run: | | ||
| set -euo pipefail | ||
| source scripts/set-test-tokens.sh | ||
| # TODO(BOT-477) Temporarily disable platform audio integration tests in CI until instability is resolved |
There was a problem hiding this comment.
should this be
# TODO(BOT-477) enable platform audio integration tests in CI when stable
?
2 participants
Our prior code coverage numbers only included unit tests. This branch updates that to also include integration tests, which are also GTest-based. Just needed to wire up the coverage results better.