Skip to content

test: exercise C++ assertions in Release CI - #102

Open
FU-max-boop wants to merge 2 commits into
XS-MLVP:masterfrom
FU-max-boop:fix/release-tests-enable-assertions-core
Open

test: exercise C++ assertions in Release CI#102
FU-max-boop wants to merge 2 commits into
XS-MLVP:masterfrom
FU-max-boop:fix/release-tests-enable-assertions-core

Conversation

@FU-max-boop

@FU-max-boop FU-max-boop commented Sep 1, 2026

Copy link
Copy Markdown

Description

Picker configures its C++ tests as a Release build. The twelve C++ test files
contain 214 standard assert(...) checks, but Release defines NDEBUG, so
those conditions are not evaluated. The workflow also directly builds and
runs only test_picker_utils, even though the current configuration registers
fourteen Picker CTests (and four xcomm tests in the normal CI build).

This change:

  • keeps Release optimization while undefining NDEBUG for targets created
    under test/;
  • adds a compile-time guard so the CI-built test_picker_utils fails closed if
    assertions become disabled again;
  • fixes two stale expectations exposed when the dormant assertions execute:
    an existing include directory is compared by filesystem identity, and the
    internal-config test expects the parser's existing map-key hierarchy;
  • replaces the one-binary workflow command with a complete CMake build and
    parallel CTest run.

Production targets retain their normal Release flags and behavior.

Related context: #54 introduced the one-binary workflow command. #96 mentioned
the path-spelling assertion that becomes active here, but did not enable
Release assertions or run the complete CTest suite. This PR does not claim to
close either item.

PR #100 was discovered during the final publication refresh. It independently
adds two parser CTests, makes the same top.sub2.key.c expectation correction,
and normalizes the path covered here by filesystem::equivalent. It does not
enable assertions in Release or replace the one-binary CI command with CTest.
A synthetic merge of #100 and this exact head is clean: the CI and
test/CMakeLists.txt changes occupy separate hunks, and Git resolves the
identical parser expectation. If #100 merges first, this branch should be
rebased so the duplicate expectation drops out and the path assertion can be
reconsidered against #100's stricter normalization guarantee; the assertion
and CTest mechanisms remain independent.

Dependencies: no hard semantic dependency; the #100 overlap is described
above and currently merges automatically.

Type of change

  • Bug fix (test and CI correctness; no production behavior change)
  • New feature
  • Breaking change
  • Documentation update

How Has This Been Tested?

Exact two-commit branch, Ubuntu 24.04 ARM64, GCC 13.3.0, CMake 3.28.3,
Release, NO_BUILD_XSPCOMM=1:

configure: status 0
complete build: status 0, 100%
CTest: 14/14 passed
effective test flags: -O3 -DNDEBUG ... -UNDEBUG

Fail-closed controls:

force -DNDEBUG after the candidate flags:
  compile status 1 at the new NDEBUG guard

replace non-primary test_parser_internalcfg_unit with /bin/false:
  old one-binary command: status 0
  targeted CTest: status 8
  restored test: status 0

A separate Ubuntu ARM64 integration stack with xcomm configured built to 100%
and passed root CTest 18/18 (four xcomm plus fourteen Picker tests). That is
coexistence evidence, not a claim that the other patches in that stack belong
to this PR.

Additional checks:

Compatibility boundary

  • GCC was exercised in the retained isolated run.
  • An earlier AppleClang run passed, but its raw log was not retained and is not
    part of the evidence claim above.
  • The MSVC /UNDEBUG path was not executed; this PR does not claim new Windows
    support.
  • In the current official configuration the new workflow reaches thirteen
    additional Picker tests and four configured xcomm tests. Future configured
    CTests will also be included by design.
  • If feat(mem_direct): export trace-derived Verilator signals #100 lands first, its two additional parser CTests will also be picked up
    automatically; the current 14-Picker/18-root counts are scoped to
    master@c100874.

Checklist

  • My code follows the style guidelines of this project.
  • I have performed a self-review of my code.
  • I have commented the non-obvious test-flag requirement.
  • My changes generate no new warning attributable to this patch.
  • I have added a fail-closed guard and execution-reachability controls.
  • New and existing configured unit tests pass with the change.
  • No downstream dependency changes are required.

@FU-max-boop FU-max-boop changed the title test: keep C++ assertions enabled in Release builds test: exercise C++ assertions in Release CI Sep 1, 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.

1 participant