Skip to content

ci: switch back to avoid build C++ tests in release builds#1817

Open
nv-nmailhot wants to merge 1 commit into
mainfrom
nmailhot/skip-tests-in-release
Open

ci: switch back to avoid build C++ tests in release builds#1817
nv-nmailhot wants to merge 1 commit into
mainfrom
nmailhot/skip-tests-in-release

Conversation

@nv-nmailhot

@nv-nmailhot nv-nmailhot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Restore the buildtype guard on the build_tests gate (reverts the #1803 change). The unit tests carry latent -Werror debt that only surfaces in release builds (-O3 -DNDEBUG, where assert() is a no-op so checked-only vars go unused and assert(0) defaults fall off non-void functions), and they aren't needed for the release wheel. Build them only in non-release buildtypes — the alternative to fixing each test's -Werror violations.

What?

Describe what this PR is doing.

Why?

Justification for the PR. If there is an existing issue/bug, please reference it. For
bug fixes, the 'Why?' and 'What?' can be merged into a single item.

How?

It is optional, but for complex PRs, please provide information about the design,
architecture, approach, etc.

Summary by CodeRabbit

  • Chores
    • Modified test build configuration to exclude tests from release builds when test building is enabled, while maintaining test builds for development builds.

Restore the buildtype guard on the build_tests gate (reverts the #1803 change).
The unit tests carry latent -Werror debt that only surfaces in release builds
(-O3 -DNDEBUG, where assert() is a no-op so checked-only vars go unused and
assert(0) defaults fall off non-void functions), and they aren't needed for the
release wheel. Build them only in non-release buildtypes — the alternative to
fixing each test's -Werror violations.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nv-nmailhot nv-nmailhot requested a review from a team as a code owner June 23, 2026 18:46
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 65aec08c-7a40-496b-a696-6bf42c559d04

📥 Commits

Reviewing files that changed from the base of the PR and between b775042 and ecf395a.

📒 Files selected for processing (1)
  • meson.build

📝 Walkthrough

Walkthrough

The Meson build gate for including the test subdirectory is updated from a single build_tests option check to a compound condition that also excludes the release buildtype. Explanatory comments are added above the new conditional.

Changes

Test Build Gate

Layer / File(s) Summary
Test subdirectory gate condition
meson.build
The if get_option('build_tests') guard is replaced with a stricter condition requiring buildtype != 'release' in addition to build_tests, with release-specific comments added above the block.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐇 A bunny checks the gate with care,
No tests shall sneak through release air!
build_tests alone won't do the trick,
release builds are locked up quick.
Hop along, the build is clean! 🏗️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description includes a clear explanation but does not follow the required template structure with distinct 'What?', 'Why?', and 'How?' sections. Restructure the description to clearly follow the template: move the existing explanation into the 'What?' section, ensure 'Why?' section addresses the rationale, and fill in 'How?' if applicable for complex changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: restoring a buildtype guard to prevent C++ tests from being built in release builds.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nmailhot/skip-tests-in-release

Comment @coderabbitai help to get the list of available commands.

@brminich

Copy link
Copy Markdown
Contributor

/build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants