You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(tests): print the real total in test_boot_log's summary - #170
tests/unit/test_boot_log.c:303 printed its summary with printf("\n%d/%d tests passed\n", tests_passed): two conversions, one argument. Every run printed 11/<garbage> tests passed (11/-1999466112 on macOS arm64). This PR prints the real total. It also makes a format warning in the unit-test programs a build error, so the next one fails the build instead of printing a wrong number.
Type of Change
feat — New feature
fix — Bug fix
docs — Documentation only
style — Formatting, no code change
refactor — Code restructuring without behavior change
test — Add or fix tests
build — Build system or dependency changes
ci — CI/CD pipeline changes
perf — Performance improvement
Changes
tests/unit/test_boot_log.c: add tests_run, incremented by RUN before each test, and print tests_passed, tests_run. A failed ASSERT calls exit(1), so the two numbers are always equal when the line prints. The point is to remove the undefined read and show the real count.
tests/CMakeLists.txt: on GCC and Clang, the unit-test programs registered in this file compile with -Werror=format. The guard reuses the target list the namespace guard above it already collects.
What gets the flag: those test programs, including the non-test sources they compile directly: core/keystore.c, stage0/jump_stage1.c, stage0/recovery_entry.c, stage0/watchdog.c and the generated test_production_key.c. All of them compile cleanly with it today.
What does not: the eboot_core, eboot_hal and eboot_stage1 libraries, every firmware or cross build (tests/ is only added for native builds, CMakeLists.txt:519), the fuzz harnesses in tests/fuzz/, and MSVC.
GCC scope: on GCC, -Werror=format also turns -Wformat-overflow and -Wformat-truncation into errors in those programs. Neither fires today (see the gcc 12.3 CI evidence below). Both depend on GCC version and optimisation level, so a newer GCC could flag an intentionally truncating snprintf in a test.
Testing
Unit tests pass (ctest --test-dir build --output-on-failure): 36/36
Integration tests pass (not run; no integration suite touched)
Manual testing performed: the summary line before and after
New tests added for new functionality: the build guard is the regression check. With it, the old test_boot_log.c fails to build at line 303.
macOS arm64, Apple clang (CommandLineTools, SDKROOT=MacOSX26.5.sdk), base 8f292909, head bdfe6d3.
master CI run 41501662283, "Build & Test (Linux x86_64)" log, grep Wformat
1 format warning in that job, and it is test_boot_log.c:303:19, which this PR fixes. Nothing else for the guard to fail on. The head build on GCC is NOT RUN locally; this PR's CI run is the evidence
Whitespace
git diff --check 8f292909 HEAD
PASS
Out of scope: the ARM cross build on master has 10 -Wformat= warnings in core/power_init.c:84, core/bmc_handoff.c:55 and core/ecc_scrub.c:65 (uint32_t is long unsigned int on newlib). This PR doesn't touch library or firmware flags; they are worth a separate issue.
Pre-Submission Checklist
Code compiles without warnings (-Wall -Wextra -Werror for C): no new warnings; the existing dev-key #warning remains
All existing tests pass
New tests added for new functionality: build guard, see Testing
Documentation updated if API changed (no API change)
Only the test harness and the test build change; no bootloader code changes. clang-cl would also report itself as Clang and get the flag. No workflow uses clang-cl.
Summary by Sourcery
Fix the boot-log test summary and prevent format-string warnings from producing misleading test results.
Bug Fixes:
Fix the boot-log unit test summary to report the actual number of tests run instead of reading a missing format argument.
Build:
Treat printf format warnings as build errors for native unit-test executables built with GCC and Clang.
Tests:
Use the unit-test build configuration as a regression check for format-string argument mismatches.
Summary by CodeRabbit
Tests
Test summaries now show the number of tests passed alongside the total number run.
Build
GNU and Clang builds apply stricter format-warning checks to applicable test executables. Other compiler types and non-executable targets are unchanged.
test_boot_log ended with printf("\n%d/%d tests passed\n", tests_passed):
two conversions, one argument. The second %d read whatever was in the
next argument register, so every run printed "11/<garbage> tests passed"
(11/-1999466112 on macOS arm64, the same on three runs). The compiler
warned about it on every build (-Wformat-insufficient-args) and nothing
failed.
Count the tests as RUN starts them and print tests_passed/tests_run.
Guard the class as well: on GCC and Clang, the unit-test programs
registered in tests/CMakeLists.txt now compile with -Werror=format, so
a format warning in a test's output fails the build. That includes the
non-test sources those programs compile directly (core/keystore.c,
stage0/jump_stage1.c, stage0/recovery_entry.c, stage0/watchdog.c and the
generated test_production_key.c). The eboot_core, eboot_hal and
eboot_stage1 libraries, every firmware/cross build and the fuzz harnesses
in tests/fuzz are unchanged. On GCC, -Werror=format also makes
-Wformat-overflow and -Wformat-truncation errors in those programs.
Fail before / pass after (macOS arm64, Apple clang, Release, tests on):
- guard + old test_boot_log.c: build fails at test_boot_log.c:303:19,
"more '%' conversions than data arguments"
- guard + this fix: build OK, 0 format warnings, summary
"11/11 tests passed", ctest 36/36
Make format warnings errors for registered GCC and Clang unit-test executables to prevent similar summary-format bugs from passing unnoticed.
tests/CMakeLists.txt L330-343
Tests
Count tests as they start and use that count in the test_boot_log summary, fixing its missing printf argument.
tests/unit/test_boot_log.c L21, L36, L305
Tips and commands
Interacting with Sourcery
Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!
No actionable comments were generated in the recent review.
Walkthrough
The test build now treats format warnings as errors for matching GNU and Clang eboot_ executables. The boot-log test counts each test invocation and uses that count in its final passed/total summary.
Changes: Test format reporting
Layer / File(s)
Summary
Format checks and test summary tests/unit/test_boot_log.c, tests/CMakeLists.txt
The boot-log test counts each RUN invocation and prints the count in its passed/total summary. GNU and Clang builds apply -Werror=format to matching eboot_ executable targets.
The test summary reports the run count, and format mismatches are guarded in eligible test builds. No merge-blocking risk was identified.
Pre-merge checks: 4 · 1
Failed checks: 1 inconclusive
Check name
Status
Explanation
Resolution
Linked Issues check
Issue #169 has two coding requirements.
1. Correct summary line (11/11 tests passed). Source evidence supports this. The head diff of tests/unit/test_boot_log.c adds static int tests_run, increment…
Provide the tests/CMakeLists.txt hunk, or the head file content after the target namespace guard, so the -Werror=format block can be checked. The check must confirm which targets receive the flag and that the eboot_core, eboot_hal and eboot…
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title names the main change: the test_boot_log summary now prints the real total. It also uses the conventional fix(tests) prefix. The objectives confirm the summary now counts tests as RUN starts…
Out of Scope Changes check
The changes reported for this PR touch two files: tests/unit/test_boot_log.c and tests/CMakeLists.txt. The test_boot_log.c change is the direct fix for the wrong summary line in #169. The tests/CMakeL…
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Title check
Explanation
The title names the main change: the test_boot_log summary now prints the real total. It also uses the conventional fix(tests) prefix. The objectives confirm the summary now counts tests as RUN starts them and prints tests_passed/tests_run, which matches the title. The title does not mention the added -Werror=format build change, but that is a secondary change, so the title remains accurate and specific.
Full details: Out of Scope Changes check
Explanation
The changes reported for this PR touch two files: tests/unit/test_boot_log.c and tests/CMakeLists.txt. The test_boot_log.c change is the direct fix for the wrong summary line in #169. The tests/CMakeLists.txt change is the build guard that #169 requests. The PR description and the supplied summary report no library, firmware, or cross-build flag changes. No unrelated change was found in the verified source. The exact extent of the CMake guard was not confirmed from source (see the linked-issue assessment). The guard is tied to #169 in any case.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.)
Full details: Linked Issues check
Explanation
Issue #169 has two coding requirements.
Correct summary line (11/11 tests passed). Source evidence supports this. The head diff of tests/unit/test_boot_log.c adds static int tests_run, increments it in RUN before each test, and prints tests_passed, tests_run. Each RUN call increments both counters, and a failed ASSERT exits before the summary. The two values match when the line prints. The 11/11 output comes from PR logs and was not rerun here.
A format or argument mismatch in a test fails the build. Not verified. The tests/CMakeLists.txt diff was unavailable because changed blobs are missing from the checkout. Reading the head file returned only the first 16000 bytes. The block after the target namespace guard, where the -Werror=format logic should be, was cut off. The base file could not be read. The AI-generated summary reports +14 lines that apply -Werror=format to GNU and Clang builds of eboot_ executable targets. That summary is not source evidence.
Resolution
Provide the tests/CMakeLists.txt hunk, or the head file content after the target namespace guard, so the -Werror=format block can be checked. The check must confirm which targets receive the flag and that the eboot_core, eboot_hal and eboot_stage1 libraries do not.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: f91eb8bc-2c6a-4a7f-b7ca-633dcfd4e2e5
Commits
Reviewing files that changed from the base of the PR and between 8f29290 and bdfe6d3.
Files selected for processing: 2
tests/CMakeLists.txt
tests/unit/test_boot_log.c
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
• Fix the boot log test summary to print the actual number of tests run.
• Make format warnings build errors for GCC and Clang unit-test executables, preventing similar
mistakes.
The scoped approach fits the defect: count tests rather than hardcode the total, and enforce format checks where the faulty output originated. Applying the flag to shared libraries or firmware would broaden the change without addressing this test-program regression.
Files changed (2) +17 / -1
Bug fix (1) +3 / -1
test_boot_log.cPrint the actual boot log test total+3/-1
Print the actual boot log test total
• Counts tests when RUN starts them and supplies that count as the missing printf argument. The summary now reports the actual passed and run totals instead of reading an undefined argument.
CMakeLists.txtMake unit-test format warnings build errors+14/-0
Make unit-test format warnings build errors
• Applies -Werror=format to executable targets registered in this directory when using GCC or Clang. The flag covers sources compiled directly into those executables without changing linked libraries, fuzz targets, or firmware builds.
Premise: pass — master tests/unit/test_boot_log.c:303 prints "%d/%d" with one argument (garbage second value); fixes #169.
Contract: n/a — test-only (tests_run counter; -Werror=format for unit-test targets).
CI: https://github.com/embeddedos-org/eBoot/pull/170/checks @ bdfe6d3 (36 checks green).
Integration: n/a.
Reviewer: loop run 2026-11-09
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
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.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Summary
tests/unit/test_boot_log.c:303printed its summary withprintf("\n%d/%d tests passed\n", tests_passed): two conversions, one argument. Every run printed11/<garbage> tests passed(11/-1999466112on macOS arm64). This PR prints the real total. It also makes a format warning in the unit-test programs a build error, so the next one fails the build instead of printing a wrong number.Type of Change
Changes
tests/unit/test_boot_log.c: addtests_run, incremented byRUNbefore each test, and printtests_passed, tests_run. A failedASSERTcallsexit(1), so the two numbers are always equal when the line prints. The point is to remove the undefined read and show the real count.tests/CMakeLists.txt: on GCC and Clang, the unit-test programs registered in this file compile with-Werror=format. The guard reuses the target list the namespace guard above it already collects.core/keystore.c,stage0/jump_stage1.c,stage0/recovery_entry.c,stage0/watchdog.cand the generatedtest_production_key.c. All of them compile cleanly with it today.eboot_core,eboot_halandeboot_stage1libraries, every firmware or cross build (tests/is only added for native builds,CMakeLists.txt:519), the fuzz harnesses intests/fuzz/, and MSVC.-Werror=formatalso turns-Wformat-overflowand-Wformat-truncationinto errors in those programs. Neither fires today (see the gcc 12.3 CI evidence below). Both depend on GCC version and optimisation level, so a newer GCC could flag an intentionally truncatingsnprintfin a test.Testing
test_boot_log.cfails to build at line 303.macOS arm64, Apple clang (CommandLineTools,
SDKROOT=MacOSX26.5.sdk), base8f292909, headbdfe6d3.cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DEBLDR_BUILD_TESTS=ON && cmake --build build -j2 && ./build/tests/eboot_test_boot_log | tail -1(base)11/-1999466112 tests passedon 3 of 3 runs; the build warns-Wformat-insufficient-argstest_boot_log.ctests/CMakeLists.txtfrom head,test_boot_log.cfrom basetest_boot_log.c:303:19: error: more '%' conversions than data arguments [-Werror,-Wformat-insufficient-args], build exit 1#warningincore/keystore.c:45)jqovercompile_commands.json-Werror=format, all in test targets; 0 of the 42eboot_core/eboot_hal/eboot_stage1library objects do./build/tests/eboot_test_boot_log | tail -111/11 tests passedctest --test-dir build --output-on-failure --no-tests=error -j2 --timeout 120grep Wformattest_boot_log.c:303:19, which this PR fixes. Nothing else for the guard to fail on. The head build on GCC is NOT RUN locally; this PR's CI run is the evidencegit diff --check 8f292909 HEADOut of scope: the ARM cross build on master has 10
-Wformat=warnings incore/power_init.c:84,core/bmc_handoff.c:55andcore/ecc_scrub.c:65(uint32_tislong unsigned inton newlib). This PR doesn't touch library or firmware flags; they are worth a separate issue.Pre-Submission Checklist
#warningremains8f292909)Related Issues
None besides the closing issue.
Closing issue
Fixes #169
Screenshots / Logs
Raw logs:
raw-run-before.log,raw-build-before.log,raw-after.log,ci-master-format-warnings.log.Additional Notes
Only the test harness and the test build change; no bootloader code changes. clang-cl would also report itself as
Clangand get the flag. No workflow uses clang-cl.Summary by Sourcery
Fix the boot-log test summary and prevent format-string warnings from producing misleading test results.
Bug Fixes:
Build:
Tests:
Summary by CodeRabbit