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
test(lifecycle): compile the app checks into test_app_lifecycle - #55
test_app_lifecycle ran 0 checks and always passed. Its checks are all under #ifdef EAPPS_BUILD_<CATEGORY>, and the build never defined those names for the compiler. This PR passes the enabled categories to the test as compile definitions and links the two apps the test needs but did not link (esettings, ebot).
It also adds two guards so the test cannot quietly go back to checking nothing, or to checking only some categories:
CMake counts the enabled categories directly from the EAPPS_BUILD_<CATEGORY> options, separately from the list of defines it passes, and hands the count to the test. The test counts the categories it actually checked and fails on any difference. Dropping one category's define now fails with got 5, expected 6.
A run in which no check ran at all fails (t_passes + t_failures > 0).
Motivation
The test is meant to catch an app whose *_info or *_lifecycle struct is missing or empty. On master it cannot fail, but every workflow that builds with BUILD_TESTING=ON counts it as a passing test. Details are in the linked issue.
Changes
tests/CMakeLists.txt:
Each enabled category adds its name to a new LIFECYCLE_DEFS list, which is passed to the test with target_compile_definitions(... PRIVATE ...).
LIFECYCLE_EXPECTED counts the categories whose option is ON and is passed as EAPPS_LIFECYCLE_EXPECTED_CATEGORIES.
esettings is linked with the productivity apps and ebot with the security apps, because the test references both.
The test is only added when at least one category is ON. Otherwise CMake prints test_app_lifecycle skipped: every EAPPS_BUILD_<CATEGORY> option is OFF.
tests/test_app_lifecycle.c:
main() counts the categories it ran, then checks that at least one check ran and that this count equals EAPPS_LIFECYCLE_EXPECTED_CATEGORIES.
An #error stops compilation if the count is not defined, so the file can't be built without it.
No CHECK_APP line changes.
CHANGELOG.md: [Unreleased] / Fixed entry, as CONTRIBUTING.md asks.
No app code changes. All 43 apps the test covers already fill in their structs correctly, so the test passes once it actually runs. etrack and the system app ehmi are built but have no CHECK_APP line, on master and here. That gap is left for a separate change.
Testing
Unit tests added/updated
Integration tests added/updated
Manual testing performed
Tested on SDL2 desktop port: SDL2 not installed here; CI runs it
Tested on EoS embedded port (if applicable)
Tested on Web/WASM port (if applicable)
Run on macOS 26 arm64, Apple clang 21.0.0, CMake 4.4.2, LVGL release/v9.2 via FetchContent, -j2, at commit 302bcc9. Log paths are shortened to <repo>.
Check
Command
Result
Master as is (84ae0fb)
ctest --test-dir _build -R test_app_lifecycle -V
Defect shown: 0 passed, 0 failed, ctest "Passed"
This branch's test file with master's tests/CMakeLists.txt
build test_app_lifecycle
FAIL as expected: #error "EAPPS_LIFECYCLE_EXPECTED_CATEGORIES must be set by tests/CMakeLists.txt", not built
This branch, EAPPS_BUILD_MEDIA define line deleted
test_app_lifecycle.c checks each category under #ifdef EAPPS_BUILD_<CATEGORY>,
but tests/CMakeLists.txt only used those names as CMake options and never
passed them to the compiler. The test compiled to an empty main(), ran 0
assertions and always passed.
Pass each enabled category as a compile definition, and link esettings and
ebot (the test references both, so the link fails without them). CMake also
counts the enabled categories from the options themselves and passes that
count in; the test fails if it checked a different number of categories, or
if no check ran at all. The test is skipped, with a status message, when
every category is off.
Configure test_app_lifecycle with the enabled category defines and required app libraries, pass the expected category count, and skip registering the test when no categories are enabled.
tests/CMakeLists.txt L47-97, L99 (removed lines)
Documentation
Document the lifecycle test fix and its safeguards in the unreleased changelog.
CHANGELOG.md L3-11
Tests
Require the expected category count at compile time and make the lifecycle test fail if it runs no checks or checks fewer categories than CMake enabled.
tests/test_app_lifecycle.c L6-9, L179-215
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 lifecycle test now receives compile definitions and libraries for enabled categories. CMake skips the test when no categories are enabled. The test fails if no checks run or if its compiled category count does not match the expected count.
Changes: Lifecycle Test
Layer / File(s)
Summary
Configure enabled categories tests/CMakeLists.txt
CMake defines each enabled category for the lifecycle test, links the required libraries, and creates and registers the test only when at least one category is enabled.
The test requires an expected category count and verifies that at least one check ran and that the compiled category count matches. The changelog records the fix.
The lifecycle test is configured to run checks for enabled categories, with no identified issue requiring a fix before merge.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: compiling lifecycle app checks into test_app_lifecycle.
Linked Issues check
Issue #54 is the only direct link. Its coding requirements are: (1) pass the EAPPS_BUILD_ options to the compiler, (2) link esettings and ebot, (3) make checks run and fail on missing or emp…
Out of Scope Changes check
The PR changes three files. Each one connects to issue #54:
- tests/CMakeLists.txt: defines, expected-category count, library links, and conditional test registration. All are needed for issue #54 re…
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: Linked Issues check
Explanation
Issue #54 is the only direct link. Its coding requirements are: (1) pass the EAPPS_BUILD_<CATEGORY> options to the compiler, (2) link esettings and ebot, (3) make checks run and fail on missing or empty *_info and *_lifecycle fields. Evidence at head 302bcc9:
Requirement 1: tests/CMakeLists.txt adds each enabled category to LIFECYCLE_DEFS and passes them with target_compile_definitions(test_app_lifecycle PRIVATE ...). The test file at head wraps each category's checks in #ifdef EAPPS_BUILD_<CATEGORY>, so these defines now enable the checks.
Requirement 2: esettings is added to LIFECYCLE_LIBS in the PRODUCTIVITY block, and ebot is added in the SECURITY block. Both match the test's references (test_productivity_apps and test_security_apps).
Requirement 3: CHECK_APP at head still asserts non-NULL and non-empty id and name, and non-NULL init and deinit. main() counts the categories it runs and asserts t_passes + t_failures > 0 and categories_checked == EAPPS_LIFECYCLE_EXPECTED_CATEGORIES. The expected count is computed from the options in CMake, separately from the define list.
The issue's scope (43 apps listed in the test) is unchanged. etrack and ehmi are excluded, as the issue requires.
No automated test run was verified by this review. The PR reports local ctest results, and CI was not run. The code evidence supports compliance.
Full details: Out of Scope Changes check
Explanation
The PR changes three files. Each one connects to issue #54:
tests/CMakeLists.txt: defines, expected-category count, library links, and conditional test registration. All are needed for issue #54 requirements 1 to 3. The skip message for the all-OFF case follows directly from the count guard.
tests/test_app_lifecycle.c: category counting, the t_passes + t_failures guard, the expected-count assertion, and the #error guard. These implement requirement 3 and the count check described in the PR. The file at head shows that CHECK_APP and the app lists are unchanged from the issue's description.
CHANGELOG.md: an [Unreleased] Fixed entry for this bug. This is documentation of the fix and does not change behavior.
No app source files are changed, as the PR states. The whole-PR diff for tests/test_app_lifecycle.c could not be read (changed blobs were unavailable). The review used the head file content and the PR summary, which reports only added lines for that file (+22/-0). This leaves no unresolved concern about out-of-scope edits.
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. (2 skipped: 2 unsupported.)
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: 37ad2e2f-0421-4ae3-899c-f4459a771916
Commits
Reviewing files that changed from the base of the PR and between 84ae0fb and 302bcc9.
Files selected for processing: 3
CHANGELOG.md
tests/CMakeLists.txt
tests/test_app_lifecycle.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.
Make app lifecycle tests run for enabled categories
🐞 Bug fix🧪 Tests🕐 10-20 Minutes
AI Description
• Compile enabled app categories into the lifecycle test so its checks actually run.
• Link required apps and fail when checks are missing or category coverage is incomplete.
• Skip the test when all categories are disabled.
Keep category definitions scoped to the lifecycle target and count expected categories from the CMake options independently. Global definitions would affect unrelated targets, while deriving the expected count from the definitions would not detect a dropped definition.
Files changed (3) +65 / -7
Bug fix (1) +34 / -7
CMakeLists.txtWire enabled categories and app libraries into the test+34/-7
Wire enabled categories and app libraries into the test
• Passes enabled category definitions and an independently calculated expected count to the lifecycle target. Links esettings and ebot for their checks, and omits the target when no categories are enabled.
test_app_lifecycle.cReject empty or incomplete category coverage+22/-0
Reject empty or incomplete category coverage
• Requires an expected category count at compile time. Tracks executed categories and fails if no app checks ran or the executed count differs from CMake’s expected count.
View medium (3) 1. Release notes omit pending additions 🐞
Description
The new ## [Unreleased] heading precedes an existing Unreleased section, while the stable-release
workflow extracts only the first such section. When that workflow publishes a release, it stops at
the intervening ## [3.0.1] heading and excludes the existing AI capability and permission-model
entries.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The new Unreleased heading causes the stable-release workflow to omit pending entries from its release notes.
## Fix Focus Areas
- CHANGELOG.md[3-11]
- CHANGELOG.md[47-57]
## Recommended Fix
Move the new Fixed entry into the existing Unreleased section and remove the newly added duplicate heading, so the workflow extracts both the existing Added entries and this fix.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The newly compiled category checks use CHECK_APP, which records a NULL id or name but then
calls strlen on that pointer without stopping. If an app has the missing metadata this test is
meant to detect, execution can terminate before TEST_EXIT summarizes the assertion failure or
checks the remaining apps.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Activating the lifecycle checks exposes a NULL dereference when an app has a missing id or name.
## Fix Focus Areas
- tests/test_app_lifecycle.c[86-102]
- tests/CMakeLists.txt[90-91]
## Recommended Fix
Make `CHECK_APP` skip each length assertion when its corresponding pointer is NULL, while retaining the NULL assertion failure and continuing to check other apps.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Adding ebot to LIFECYCLE_LIBS makes the security-enabled lifecycle executable pull in ebot.c,
which calls Winsock functions without a CMake link dependency on ws2_32. On Windows GCC builds,
the source's MSVC-specific link pragma does not supply that library, leaving the test executable
with unresolved socket symbols.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The new ebot dependency introduces unresolved Winsock references in Windows GCC lifecycle-test builds.
## Fix Focus Areas
- tests/CMakeLists.txt[78-80]
- apps/ebot/CMakeLists.txt[7-13]
## Recommended Fix
Add a Windows-conditional `ws2_32` link dependency to the ebot target so executables consuming the static library, including the lifecycle test, receive it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The new ## [Unreleased] heading precedes an existing Unreleased section, while the stable-release
workflow extracts only the first such section. When that workflow publishes a release, it stops at
the intervening ## [3.0.1] heading and excludes the existing AI capability and permission-model
entries.
Agent Prompt
## Issue description
The new Unreleased heading causes the stable-release workflow to omit pending entries from its release notes.
## Fix Focus Areas
- CHANGELOG.md[3-11]
- CHANGELOG.md[47-57]
## Recommended Fix
Move the new Fixed entry into the existing Unreleased section and remove the newly added duplicate heading, so the workflow extracts both the existing Added entries and this fix.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The reason will be displayed to describe this comment to others. Learn more.
2. Missing app names crash the lifecycle test 🐞 Bug☼ Reliability
The newly compiled category checks use CHECK_APP, which records a NULL id or name but then
calls strlen on that pointer without stopping. If an app has the missing metadata this test is
meant to detect, execution can terminate before TEST_EXIT summarizes the assertion failure or
checks the remaining apps.
Agent Prompt
## Issue description
Activating the lifecycle checks exposes a NULL dereference when an app has a missing id or name.
## Fix Focus Areas
- tests/test_app_lifecycle.c[86-102]
- tests/CMakeLists.txt[90-91]
## Recommended Fix
Make `CHECK_APP` skip each length assertion when its corresponding pointer is NULL, while retaining the NULL assertion failure and continuing to check other apps.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The reason will be displayed to describe this comment to others. Learn more.
3. Windows gcc builds cannot link the test 🐞 Bug≡ Correctness
Adding ebot to LIFECYCLE_LIBS makes the security-enabled lifecycle executable pull in ebot.c,
which calls Winsock functions without a CMake link dependency on ws2_32. On Windows GCC builds,
the source's MSVC-specific link pragma does not supply that library, leaving the test executable
with unresolved socket symbols.
Agent Prompt
## Issue description
The new ebot dependency introduces unresolved Winsock references in Windows GCC lifecycle-test builds.
## Fix Focus Areas
- tests/CMakeLists.txt[78-80]
- apps/ebot/CMakeLists.txt[7-13]
## Recommended Fix
Add a Windows-conditional `ws2_32` link dependency to the ebot target so executables consuming the static library, including the lifecycle test, receive it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments### Comment 1
<locationpath="tests/CMakeLists.txt"line_range="79" />
<code_context>
endif()
if(EAPPS_BUILD_SECURITY)
- list(APPEND LIFECYCLE_LIBS evpn eguard evirustower)
+ list(APPEND LIFECYCLE_LIBS evpn eguard evirustower ebot)
+ list(APPEND LIFECYCLE_DEFS EAPPS_BUILD_SECURITY)
endif()
</code_context>
<issue_to_address>
**Lifecycle test cannot link**
When `EAPPS_BUILD_SECURITY` is ON and no `lvgl` target is available, `tests/CMakeLists.txt` appends `ebot` to `LIFECYCLE_LIBS` while `apps/ebot/CMakeLists.txt` skips creating that target, so the lifecycle test fails to link and the build fails.
Only append `ebot` to `LIFECYCLE_LIBS` when its target is created.
</issue_to_address>
Linked issues
#54 [BUG] test_app_lifecycle runs 0 checks and always passes · 2 of 2 met
#
Requirement
Status
Notes
1
Run the lifecycle checks for every enabled app category, checking each listed app’s info and lifecycle fields.
✅ Met
CMake passes each enabled category define to the test; each app’s CHECK_APP performs the six requested checks, and the test verifies that at least one check ran and that all enabled categories were checked.
2
Link the missing esettings and ebot libraries when their checks are compiled.
✅ Met
The productivity and security library lists now include esettings and ebot, respectively.
Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: tests/CMakeLists.txt:79
Sourcery is free for open source - if you like our reviews please consider sharing them ✨
The reason will be displayed to describe this comment to others. Learn more.
🟡 Medium · Lifecycle test cannot link
When EAPPS_BUILD_SECURITY is ON and no lvgl target is available, tests/CMakeLists.txt appends ebot to LIFECYCLE_LIBS while apps/ebot/CMakeLists.txt skips creating that target, so the lifecycle test fails to link and the build fails.
Only append ebot to LIFECYCLE_LIBS when its target is created.
Prompt for AI agents
In `tests/CMakeLists.txt` at line 79:
**Lifecycle test cannot link**
When `EAPPS_BUILD_SECURITY` is ON and no `lvgl` target is available, `tests/CMakeLists.txt` appends `ebot` to `LIFECYCLE_LIBS` while `apps/ebot/CMakeLists.txt` skips creating that target, so the lifecycle test fails to link and the build fails.
Only append `ebot` to `LIFECYCLE_LIBS` when its target is created.
Premise: pass — master tests/CMakeLists.txt:66-71 never passed EAPPS_BUILD_* defines (0 assertions); SECURITY lacked ebot, PRODUCTIVITY lacked esettings; fixes #54.
Contract: pass — lifecycle test #errors without the count macro and fails on 0 checks or count mismatch.
CI: https://github.com/embeddedos-org/eApps/pull/55/checks @ 302bcc9 (22 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.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Pull Request
Closing issue
Fixes #54
Description
test_app_lifecycleran 0 checks and always passed. Its checks are all under#ifdef EAPPS_BUILD_<CATEGORY>, and the build never defined those names for the compiler. This PR passes the enabled categories to the test as compile definitions and links the two apps the test needs but did not link (esettings,ebot).It also adds two guards so the test cannot quietly go back to checking nothing, or to checking only some categories:
EAPPS_BUILD_<CATEGORY>options, separately from the list of defines it passes, and hands the count to the test. The test counts the categories it actually checked and fails on any difference. Dropping one category's define now fails withgot 5, expected 6.t_passes + t_failures > 0).Motivation
The test is meant to catch an app whose
*_infoor*_lifecyclestruct is missing or empty. On master it cannot fail, but every workflow that builds withBUILD_TESTING=ONcounts it as a passing test. Details are in the linked issue.Changes
tests/CMakeLists.txt:LIFECYCLE_DEFSlist, which is passed to the test withtarget_compile_definitions(... PRIVATE ...).LIFECYCLE_EXPECTEDcounts the categories whose option is ON and is passed asEAPPS_LIFECYCLE_EXPECTED_CATEGORIES.esettingsis linked with the productivity apps andebotwith the security apps, because the test references both.test_app_lifecycle skipped: every EAPPS_BUILD_<CATEGORY> option is OFF.tests/test_app_lifecycle.c:main()counts the categories it ran, then checks that at least one check ran and that this count equalsEAPPS_LIFECYCLE_EXPECTED_CATEGORIES.#errorstops compilation if the count is not defined, so the file can't be built without it.CHECK_APPline changes.CHANGELOG.md:[Unreleased]/ Fixed entry, asCONTRIBUTING.mdasks.No app code changes. All 43 apps the test covers already fill in their structs correctly, so the test passes once it actually runs.
etrackand the system appehmiare built but have noCHECK_APPline, on master and here. That gap is left for a separate change.Testing
Run on macOS 26 arm64, Apple clang 21.0.0, CMake 4.4.2, LVGL
release/v9.2via FetchContent,-j2, at commit302bcc9. Log paths are shortened to<repo>.84ae0fb)ctest --test-dir _build -R test_app_lifecycle -V0 passed, 0 failed, ctest "Passed"tests/CMakeLists.txttest_app_lifecycle#error "EAPPS_LIFECYCLE_EXPECTED_CATEGORIES must be set by tests/CMakeLists.txt", not builtEAPPS_BUILD_MEDIAdefine line deletedctest -R test_app_lifecycle -Vgot 5, expected 6;229 passed, 1 failed; ctest exit 8EAPPS_BUILD_SECURITYdefine line deletedgot 5, expected 6;235 passed, 1 failed; ctest exit 8at least one app check ran;got 0, expected 6); ctest exit 8260 passed, 0 failed(43 apps × 6 checks = 258, plus the 2 guards)esettings/ebottest_app_lifecycle_ebot_info,_ebot_lifecycle,_esettings_info,_esettings_lifecycle(this is why both are now linked)test_app_lifecycle26 passed/86 passed, 0 failed, exit 0cmake -DBUILD_TESTING=ON -DEAPPS_BUILD_{PRODUCTIVITY,MEDIA,GAMES,CONNECTIVITY,SECURITY,WEB}=OFF,ctest -Ncmake --build _build -j2ctest --test-dir _build --output-on-failureclang-format --dry-run --Werror --lines=...on every added range oftests/test_app_lifecycle.c(clang-format 23.1.5, repo.clang-format)git diff --check HEAD~1 HEADcppcheck,clang-tidyThe image shows master, the new test file against master's CMake, the dropped-define case and this branch:
Checklist
ctest --test-dir build --output-on-failure)clang-format): added lines onlycppcheck,clang-tidy): NOT RUN, see aboveSummary by Sourcery
Ensure the app lifecycle test validates all enabled application categories and cannot silently pass without running its checks.
Bug Fixes:
Enhancements:
Build:
Tests:
Chores:
Summary by CodeRabbit