Skip to content

ci: isolate kvrocks2redis coverage data#3511

Closed
jihuayu wants to merge 1 commit into
apache:unstablefrom
jihuayu:codex/test-sonar-kvrocks2redis-coverage
Closed

ci: isolate kvrocks2redis coverage data#3511
jihuayu wants to merge 1 commit into
apache:unstablefrom
jihuayu:codex/test-sonar-kvrocks2redis-coverage

Conversation

@jihuayu

@jihuayu jihuayu commented Jun 3, 2026

Copy link
Copy Markdown
Member

The previous go test code coverage data could be overwritten by the coverage data from kvrocks2redis, which caused the overall coverage metric to be lower than expected.

This fixes the SonarCloud coverage job so the kvrocks2redis test no longer corrupts gcov data generated by the main C++ and Go tests.

Previously, the coverage job ran the normal test suite and then ran the kvrocks2redis test from the same coverage build. Both kvrocks and kvrocks2redis link kvrocks_objs, so they wrote to the same .gcda files. This produced libgcov profiling error: overwriting an existing profile data with a different timestamp and could make SonarCloud report some covered files as uncovered.

The fix keeps the coverage data isolated:

  • collect the main C++/Go coverage into coverage-main.json
  • run the kvrocks2redis source server from a separate coverage build directory
  • run the kvrocks2redis tool itself from a non-coverage build directory to avoid writing conflicting kvrocks_objs profiles
  • stop daemonized test processes before collecting coverage
  • merge both tracefiles into one SonarQube XML report

This does not add new command coverage by itself. It makes sure coverage produced by existing tests is preserved and merged correctly instead of being overwritten by the kvrocks2redis stage.

Assisted-by: Codex/GPT5.5 xhigh

@jihuayu jihuayu marked this pull request as ready for review June 3, 2026 13:23
@jihuayu jihuayu requested review from PragmaTwice and git-hulk June 3, 2026 13:23
@jihuayu

jihuayu commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

The reason for this PR is that I noticed the coverage report in #3502 looked strange. The coverage for src/commands/cmd_stream.cc should not have been 0.

@PragmaTwice PragmaTwice left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not acceptable to me that the CI time increases by about 35min (1h 24m 59s in total).

See https://github.com/apache/kvrocks/actions/runs/26887545442/job/79334883335?pr=3511.

@jihuayu

jihuayu commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

Ummm, I overlooked that. This is indeed unacceptable. I decided to parallelize the build steps for kvrocks2redis and Kvrocks.

Do you have a better approach?

@PragmaTwice

Copy link
Copy Markdown
Member

You can just disable the kvrocks2redis test in such matrix config.. Please do not increase CI time. It's already too slow..

@sonarqubecloud

sonarqubecloud Bot commented Jun 3, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
40.0% Coverage on New Code (required ≥ 50%)

See analysis details on SonarQube Cloud

@jihuayu jihuayu closed this Jun 4, 2026
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.

2 participants