Skip to content

Split protocol_test.cc into four translation units - #424

Merged
philipcraig merged 7 commits into
mainfrom
split-protocol-test-423
Sep 18, 2026
Merged

philipcraig merged 7 commits into
mainfrom
split-protocol-test-423

Conversation

@philipcraig

@philipcraig philipcraig commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Part of #423.

Splits protocol_test.cc by topic so that a change to one topic recompiles 11 to 21 s of tests rather than 50 s. This is for incremental rebuilds; it is not expected to change CI times, and the CI figures below show none. The tests themselves are unchanged.

Per-TU compile time, -O2 -c, GCC trunk 20260906 on a 2-core Codespaces VM:

TU Time
protocol_test.cc (before) 50.5 s
protocol_test.cc 15.9 s
protocol_view_tests.cc 20.6 s
protocol_member_function_tests.cc 17.5 s
conformance_tests.cc 11.3 s

A full build does about 15 s more work, because each new TU pays the 4.8 s gtest and protocol.hh baseline again. That is why CI does not get faster: the shorter critical path only helps on a machine with more cores than TUs.

CI, one run each, main (f01e297, scheduled) against this PR. The CMake figures are the ninja build span and, in brackets, the time until the last test object finished, both from log timestamps; these jobs have no build cache. The Bazel and sanitizer jobs restore a disk cache, so they are not compared. The differences are within run-to-run variation.

Job main this PR
CMake, GCC trunk Release 73 s (61 s) 88 s (58 s)
CMake, GCC trunk Debug 51 s (51 s) 50 s (50 s)
CMake, GCC-16 Release 57 s (48 s) 56 s (36 s)
CMake, GCC-16 Debug 28 s (27 s) 36 s (35 s)
CMake, GCC-16 macOS Release 35 s (30 s) 39 s (33 s)
Runtime coverage, build and test step 86 s 92 s
Consteval coverage, probe step 205 s 283 s

The consteval coverage script names its TUs, so its list gains conformance_tests.cc and protocol_view_tests.cc; protocol_member_function_tests.cc reaches no probe point the others miss and is left out. Coverage is unchanged at 194 of 216 probe points. Run back to back on the 2-core VM the probe takes 7m26 on main and 6m52 here; the CI probe step is slower in this one run, where #425, which has main's TU list, took 243 s.

#421 and #356 add tests to protocol_test.cc in regions that move, so whichever lands second needs its new tests re-homed.

protocol_test.cc was the longest test TU to compile and set the floor for
a parallel build. Move the conformance tests, the protocol_view member
function and views-of-protocols tests, and the protocol member function
tests into their own files. Tests are moved unchanged.

Part of #423.
@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.21%. Comparing base (6b86c62) to head (b2c0f1e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #424   +/-   ##
=======================================
  Coverage   81.21%   81.21%           
=======================================
  Files          16       16           
  Lines        1299     1299           
  Branches      214      214           
=======================================
  Hits         1055     1055           
  Misses         45       45           
  Partials      199      199           
Flag Coverage Δ
consteval 90.04% <ø> (ø)
runtime 79.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@philipcraig
philipcraig marked this pull request as ready for review September 18, 2026 13:58
@philipcraig
philipcraig requested a review from jbcoe as a code owner September 18, 2026 13:58
@philipcraig
philipcraig enabled auto-merge (squash) September 18, 2026 13:58
The conversion function conformance tests added to protocol_test.cc on
main move to conformance_tests.cc. protocol_operator_tests.cc joins the
consteval coverage translation units ahead of protocol_test.cc, as on
main.
@philipcraig
philipcraig merged commit c98c808 into main Sep 18, 2026
21 checks passed
@philipcraig
philipcraig deleted the split-protocol-test-423 branch September 18, 2026 22:19
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