Add regression test for init template defaults (#747)#1016
Add regression test for init template defaults (#747)#1016malikkaraoui wants to merge 1 commit intoholepunchto:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a regression test + fixture to ensure pear init uses _template.json-provided defaults (not hardcoded fallbacks), preventing a recurrence of #747.
Changes:
- Add
test/14-init.test.jsto validate template defaults are honored duringinit()withautosubmit: true - Add
test/fixtures/custom-template-defaults/fixture template definingheight/widthdefaults - Register the new test in
test/index.js
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/index.js | Includes the new init regression test in the test runner. |
| test/fixtures/custom-template-defaults/package.json | Adds a template output file using stamp placeholders for name/height/width. |
| test/fixtures/custom-template-defaults/_template.json | Defines template params/defaults (including height/width) to reproduce #747. |
| test/14-init.test.js | New regression test asserting init output uses template defaults. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const output = await init(templateDir, outDir, { | ||
| cwd: os.cwd(), | ||
| ipc: helper, | ||
| autosubmit: true, | ||
| defaults: { name: 'test-app' }, | ||
| header: '', | ||
| force: true, | ||
| pkg: null |
There was a problem hiding this comment.
This test doesn’t currently reproduce the regression scenario from #747: the passed defaults only includes name, so even a precedence bug where passed defaults override template defaults for height/width wouldn’t be caught. To make this a true regression test, pass height: 540 and width: 720 in defaults (to mimic the previous hardcoded fallbacks) and assert the generated package.json still uses the template defaults (1080/800).
malikkaraoui
force-pushed
the
test/init-template-defaults-747
branch
from
cf923ce to
ef76c26
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "height": "__height__", | ||
| "width": "__width__" |
There was a problem hiding this comment.
In this fixture, height/width placeholders are quoted, so the generated package.json will have pear.gui.height/width as strings. If these fields are intended to be numbers (as in examples/desktop/package.json), consider removing the quotes around __height__ / __width__ and updating the test assertions accordingly so the regression test matches real-world config types.
| "height": "__height__", | |
| "width": "__width__" | |
| "height": __height__, | |
| "width": __width__ |
| for await (const msg of output) { | ||
| if (msg.tag === 'final') break | ||
| } |
There was a problem hiding this comment.
The test breaks out of the output stream loop at the final tag but doesn’t assert final.data.success === true or fail on any error tags. Consider capturing the final message and asserting success (and/or scanning for error output) to avoid silent partial failures where package.json happened to be written but the init run still failed.
The hardcoded height/width fallbacks (540/720) in cmd/init.js were removed during the v2 refactor. Add a test that passes the old hardcoded values in defaults and verifies that template defaults (1080/800) take precedence, preventing the original regression.
malikkaraoui
force-pushed
the
test/init-template-defaults-747
branch
from
ef76c26 to
31d8e89
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 participant
Summary
_template.jsondefaults are used bypear init, not overridden by hardcoded fallback valuesContext
Issue #747 reported that hardcoded
height: 540andwidth: 720fallbacks incmd/init.jswere overriding custom template defaults. This was resolved during the v2 refactor — thedefaultsobject now only contains{ name }, andinit/index.jscorrectly prioritizes template defaults:However, no test existed to prevent regression. This PR adds one.
What's included
test/fixtures/custom-template-defaults/— a template withheight: 1080andwidth: 800defaultstest/14-init.test.js— callsinit()withautosubmit: trueand verifies:nameuses the passed default (test-app)heightuses the template default (1080, not540)widthuses the template default (800, not720)Test plan
npm run lintpassesnpm testpasses with new test included