Generate a single launcher for manual, automated, and stress test plans (Bugfix) - #2819
Merged
rickwu666666 merged 13 commits intoSep 18, 2026
Merged
Conversation
Previously, three separate launchers were generated for manual, automated, and stress test plans, which could confuse ODM users about which launcher to use.
rickwu666666
added
the
ce-oem
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The manual/auto/stress selection logic is currently too loose (suffix-only matching) and can select unintended nested plans, and there is also a small correctness issue in progress output messaging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the CE-OEM launcher generator (in contrib/checkbox-ce-oem/.../checkbox-launcher-generator) to produce a single merged launcher for top-level plans that nest the manual/automated/stress trio, reducing ambiguity for ODM users.
Changes:
- Add
select_manual_auto_stress()to detect and order the manual/automated/stress trio for a top-level plan. - Extend
write_launcher()to support writing a[test plan] filterlist and to toggleforced/UI type accordingly. - Update the TUI save flow and documentation, and add unit tests covering the merged-launcher behavior.
File summaries
| File | Description |
|---|---|
| contrib/checkbox-ce-oem/checkbox-ce-oem-snap/checkbox-launcher-generator/tests/test_gen_launcher.py | Adds tests for new launcher output (filter/forced) and for the merged-save behavior. |
| contrib/checkbox-ce-oem/checkbox-ce-oem-snap/checkbox-launcher-generator/tests/test_checkbox_ce_oem_scan.py | Adds tests for manual/automated/stress selection and ordering. |
| contrib/checkbox-ce-oem/checkbox-ce-oem-snap/checkbox-launcher-generator/src/gen_launcher.py | Updates launcher writing and TUI save logic to emit one merged launcher for manual/auto/stress trios. |
| contrib/checkbox-ce-oem/checkbox-ce-oem-snap/checkbox-launcher-generator/src/checkbox_ce_oem_scan.py | Introduces select_manual_auto_stress() helper to filter and order the trio. |
| contrib/checkbox-ce-oem/checkbox-ce-oem-snap/checkbox-launcher-generator/README.md | Documents the new merged-launcher behavior and updates the unit test count. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: rickwu666666 <98441647+rickwu666666@users.noreply.github.com>
Long environ/manifest values wrapped the row's urwid.Text onto a second line, but ListBox had cached the row height as 1, crashing with ListBoxError: calculated 1 rows but rendered 2. Add wrap="clip" to the display Text and edit-mode Edit widget so a row is always exactly one line, matching every other Text widget in this file. Truncate long values with a leading ellipsis (mirroring the existing key truncation) so users still see the tail of the value; the full value remains visible when editing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rickwu666666
marked this pull request as draft
September 8, 2026 10:38
Contributor
Author
rickwu666666
marked this pull request as ready for review
September 8, 2026 10:56
- write_launcher() no longer emits a shebang line into generated launcher files, since they are not meant to be executed directly. - Add an ini-based launcher template mechanism (load_launcher_template()/launcher_template.ini) so every launcher section other than [launcher], [test plan], [manifest] and [environment] (e.g. [ui], [restart], [report:...], [transport:...]) can be maintained in one editable ini file instead of being hard-coded in gen_launcher.py. - Support --template PATH and $CHECKBOX_LAUNCHER_TEMPLATE as an external override, since the script's own install location is read-only once packaged into a snap. - Bundle a default launcher_template.ini next to gen_launcher.py and stage it in all 8 series' snapcraft.yaml files. - Add/update tests in test_gen_launcher.py and document the new behaviour in README.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rickwu666666
marked this pull request as draft
September 9, 2026 05:57
Change the built-in fallback for [ui] type in write_launcher() (used when neither the template nor a manual/automated/stress filter picker sets it) from 'silent' to 'interactive', and update the bundled launcher_template.ini default to match. Templates can still override type = silent for unattended runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
write_launcher() no longer hard-codes any fallback/override for [ui] or any other non-fixed section (previously it defaulted/forced [ui] type based on *forced*). Every section other than [launcher], [test plan], [manifest], and [environment] is now copied verbatim from template_sections with no special-casing, including [ui] — a missing template simply means that section isn't written at all. launcher_template.ini's 'type = interactive' is what makes the manual/automated/stress test-plan picker actually work; getting that right is now entirely the template's responsibility, not gen_launcher.py's. Update tests and README to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
checkbox-ng's own [test plan] forced VarSpec already defaults to False (plainbox/impl/config.py), so write_launcher() now only writes 'forced = yes' when forced=True and omits the line entirely otherwise, instead of spelling out 'forced = no'. Also confirms/documents (with a strengthened test) that a template's own [test plan] section — forced = yes included — is dropped in full by load_launcher_template()'s forbidden-section filtering before it ever reaches write_launcher(), so there is no merge/conflict with the [test plan] section gen_launcher.py generates itself. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The launcher file is just an ini document, so the template now follows normal ini semantics instead of a fixed allow/forbid list: - load_launcher_template() loads every section from the template file, with no section skipped. - write_launcher() builds the generated [launcher]/[test plan]/ [manifest]/[environment] sections first, then merges the template on top per-section/per-key: a matching section+key overrides the generated value, a new key in an existing section is added, and an entirely new section (e.g. [ui]) is appended as-is. - The forced parameter and all forced-related code are removed from write_launcher(); forced is now purely a template concern, added via the template's own [test plan] section if desired. Updated tests and README to match.
rickwu666666
marked this pull request as ready for review
September 9, 2026 09:08
rickwu666666
marked this pull request as draft
September 9, 2026 09:08
Contributor
Author
|
build successfully |
rickwu666666
marked this pull request as ready for review
September 10, 2026 06:22
Contributor
Author
|
build successfully. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Address the two moderate generator issues; template comment inconsistencies are also noted.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
contrib/checkbox-ce-oem/checkbox-ce-oem-snap/checkbox-launcher-generator/src/gen_launcher.py:290
- This removes the shebang from every generated launcher, so the output can no longer be executed the way existing generated/provider launchers are used (
./<plan-id>-launcher). The PR only needs to merge the plans into one file; please preserve the existing direct-execution contract by retaining the wrapper shebang (and keep it outside the INI sections).
lines: list[str] = []
for section, kv in sections.items():
if lines:
lines.append("")
lines.append(f"[{section}]")
for key, val in kv.items():
lines.append(f"{key} = {val}")
output_path.write_text("\n".join(lines) + "\n", encoding="utf-8")
contrib/checkbox-ce-oem/checkbox-ce-oem-snap/checkbox-launcher-generator/src/launcher_template.ini:24
- This comment says
gen_launcher.pyforces[ui] typetointeractivewhenever it writes a filtered plan, but the implementation only merges the template and never overrides that value; a custom template withtype = silentproduces a silent launcher. Please describe this as the bundled template's default rather than behavior enforced by the generator.
# gen_launcher.py always forces this to "interactive" when it writes a
# [test plan] filter (manual/automated/stress selection), regardless of
# what's set here, since the picker prompt requires it. For every other
contrib/checkbox-ce-oem/checkbox-ce-oem-snap/checkbox-launcher-generator/src/launcher_template.ini:8
- This bundled template comment contradicts the implementation and the README: generated sections are not ignored;
write_launcher()merges template keys into[launcher],[test plan],[manifest], and[environment], including overrides. As written, maintainers may believe these sections are ineffective and avoid relying on supported overrides.
# Every section here (except [ui]'s "type", see below) is copied verbatim
# into every launcher gen_launcher.py writes, in the order given. Only
# [launcher], [test plan], [manifest] and [environment] are off-limits —
# those are always generated by gen_launcher.py itself from the selected
# plan and the values filled in the editor, so any such section here is
# silently ignored.
- Files reviewed: 14/14 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
rickwu666666
deleted the
generate-one-launcher-from-launcher-generator
branch
September 18, 2026 06:46
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.
Previously, three separate launchers were generated for manual, automated, and stress test plans, which could confuse ODM users about which launcher to use.
Description
We make the generated launcher unify to one launcher instead of three launchers. And the launcher will include the manual, automated and stress test plans. e.g.
Resolved issues
Documentation
Tests
All build passed.
https://github.com/canonical/checkbox/actions/runs/33603389661