style: bring native_tests under the clang-format gate - #13
Merged
Conversation
The CI clang-format job globbed `find src include tests`, but tests/ was renamed native_tests/ in 0.2.2. find printed an error and moved on, and because the pipeline's exit status comes from clang-format rather than find, the job stayed green while silently checking nothing in the native test suite. It had therefore never been formatted, and carried 1233 violations under both clang-format 18 and 22. Add native_tests to CXX_DIRS in scripts/format.sh — which #12 made the single source of truth for both CI jobs — and apply clang-format 18.1.8, the pinned version. This is a mechanical reformat with no behavior change. Verified per file: every body's token stream is byte-identical with whitespace removed, and every #include set is unchanged as a multiset, so the only differences are line wrapping and include ordering (clang-format's SortIncludes). The C++ suite still builds and passes 149/149, the same count as before.
jwinarske
force-pushed
the
jw/format-native-tests
branch
from
August 14, 2026 16:25
a052b60 to
b5d0bc2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #12 — review that first; this PR targets its branch so the diff shows only the reformat. Rebases onto
mainonce #12 merges.The gap
CI's clang-format job globbed
find src include tests, buttests/was renamednative_tests/in 0.2.2.findprinted'tests': No such file or directoryand carried on, and because a pipeline's exit status comes from its last command —clang-format, notfind— the job stayed green while silently checking nothing in the native test suite.So
native_tests/had never been formatted. It carried 1233 violations, under clang-format 18 and 22 alike.The change
native_testsjoinsCXX_DIRSinscripts/format.sh(#12 made that the single source of truth for both CI jobs), and clang-format 18.1.8 — the pinned version — is applied.This is mechanical
The diff is ~1000 lines, so the no-behavior-change claim is verified rather than asserted. Per file:
#includesets are identical as multisets, so nothing was added or droppedThat leaves exactly two kinds of difference: line wrapping, and include ordering from clang-format's
SortIncludes. The C++ suite rebuilds and passes 149/149 — the same count as before, so no test was accidentally renamed out of discovery.Follow-up
scripts/test.shand.github/workflows/ci.ymlstill each spell out their own CMake flags. That duplication is what let theBUILD_TESTING/APPSTREAM_BUILD_TESTSmismatch survive in 0.4.0 and thistests/path survive since 0.2.2. Worth collapsing the same way formatting just was, but not in this PR.