Skip to content

fix(ci): isolate LLVM profile outputs#20

Merged
mrmidi merged 2 commits into
mrmidi:mainfrom
gly11:fix/ci-unique-llvm-profile
May 28, 2026
Merged

fix(ci): isolate LLVM profile outputs#20
mrmidi merged 2 commits into
mrmidi:mainfrom
gly11:fix/ci-unique-llvm-profile

Conversation

@gly11

@gly11 gly11 commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR makes the C++ coverage step deterministic when tests run in parallel. The workflow currently lets every instrumented test process write to the default default.profraw, which can produce mismatched or corrupted profile data under ctest -j.

Setting LLVM_PROFILE_FILE to include process/module placeholders gives each test process its own profile output before llvm-profdata merge.

Context

This showed up on #19 after all 480 tests passed, when coverage processing failed with:

warning: build/tests/default.profraw: unrecognized instrumentation profile encoding format
error: no profile can be merged

A previous successful run on the same source tree already reported mismatched profile data, so this is a CI coverage stability fix rather than a product-code change.

Verification

git diff --check
ruby -e 'require "yaml"; YAML.load_file(".github/workflows/build-and-test.yml"); puts "yaml ok"'

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ad37d4a5e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/build-and-test.yml Outdated
run: |
cd build/tests
ctest --output-on-failure -j$(sysctl -n hw.ncpu)
LLVM_PROFILE_FILE="$PWD/%p-%m.profraw" ctest --output-on-failure -j$(sysctl -n hw.ncpu)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Isolate profile output before test discovery runs

This only sets LLVM_PROFILE_FILE for the ctest invocation, but the preceding Build Tests step also runs many instrumented test executables because tests/CMakeLists.txt uses gtest_discover_tests(...) throughout, and CMake's default DISCOVERY_MODE is POST_BUILD (it runs each binary with --gtest_list_tests at build time). Under cmake --build ... -j, those discovery processes can still concurrently write build/tests/default.profraw, and the later llvm-profdata merge build/tests/*.profraw will include that same potentially corrupted file, so the CI coverage failure this patch targets can still occur before this environment assignment ever applies.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@codex review. Fixed in 2864d80

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Codex Review: Didn't find any major issues. Keep them coming!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@mrmidi

mrmidi commented May 28, 2026

Copy link
Copy Markdown
Owner

Thanks, looks valid

@mrmidi mrmidi merged commit a409340 into mrmidi:main May 28, 2026
2 checks passed
mrmidi added a commit that referenced this pull request Jun 12, 2026
fix(ci): isolate LLVM profile outputs
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