Skip to content

test(desktop): validate release arguments against the Electron Builder schema - #168

Merged
elkaix merged 1 commit into
mainfrom
test/eb-config-schema-validation
Aug 23, 2026
Merged

test(desktop): validate release arguments against the Electron Builder schema#168
elkaix merged 1 commit into
mainfrom
test/eb-config-schema-validation

Conversation

@elkaix

@elkaix elkaix commented Aug 23, 2026

Copy link
Copy Markdown
Member

Related Issue

No issue — follow-up hardening for the release failure fixed in #167.

Problem

The desktop-v0.2.1 build died at Electron Builder's schema validation on --config.win.publisherName, an option removed in v26. The suite was green throughout, because package-win.spec.ts compared the generated argument array to a hand-written expected array. Both copies carried the same mistake, so the test only detected change, never invalidity — Google's "change-detector test".

The consequence is structural, not incidental: these arguments are only emitted when signing secrets are present, so a tag build is the first time they ever execute. A test that cannot judge them means the release is the first judge.

What changed

package-win.spec.ts now merges the generated --config.* arguments onto the packaged build configuration and validates the result with Ajv against the installed app-builder-lib/scheme.json, using the same Ajv options app-builder-lib itself validates with (allErrors, verbose, coerceTypes, strict: false).

  • No new dependency. Both scheme.json and ajv resolve through the existing electron-builder dependency (pnpm hides them from a direct resolve out of apps/desktop, so resolution is anchored at require.resolve('electron-builder')).
  • Follows the installed version. It reads whatever schema the pinned Electron Builder ships, so a future major that relocates these options fails here rather than at a tag.
  • Failure output is trimmed to the actionable errors instead of Ajv's verbose dump.

Three cases: the packaged configuration alone, the configuration each signing method produces, and a negative case pinning the exact regression.

Verification

Mutation-tested, both caught:

Mutation Result
Reintroduce --config.win.publisherName 3 failed — Electron Builder rejects unknown options: /win.publisherName
Drop required azureSignOptions.certificateProfileName 3 failed — must have required property 'certificateProfileName'
Restored 17 passed

The second case is a class the previous test could not detect at all.

  • pnpm exec vitest run tests/package-win.spec.ts — 17 passed
  • pnpm run typecheck (apps/desktop) — clean, both tsconfigs
  • oxlint --type-aware — 0 warnings, 0 errors
  • scripts/check-no-comments.mjs — OK

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked a related issue — none; follow-up to fix(desktop): correct the Windows and macOS release verification #167.
  • I have added tests that prove my feature works.
  • Ran gen-changesets skill, or this PR needs no changeset — no changeset: test-only, nothing users can perceive.
  • Ran gen-docs skill, or this PR needs no doc update.

Summary by CodeRabbit

  • Tests
    • Improved validation of desktop package configurations against the Electron Builder schema.
    • Added coverage for base configurations, signing modes, invalid options, and removed settings.
    • Enhanced error reporting for unknown or invalid configuration values.

…r schema

The 0.2.1 Windows build died at schema validation on an option Electron
Builder 26 had removed, and the suite was green because it compared the
generated arguments to a hand-written array — the same mistake written
twice. That shape of test cannot tell that arguments are invalid, only that
they changed.

Merge the generated `--config.*` arguments onto the packaged build
configuration and validate the result with Ajv against the installed
`app-builder-lib/scheme.json`, under the same Ajv settings Electron Builder
uses. Both resolve through the existing electron-builder dependency, so this
adds nothing to the lockfile and follows whatever schema the installed
version ships — including a future major that moves these options again.

It catches more than the original defect. Verified by mutation: restoring
`--config.win.publisherName` fails three tests with "rejects unknown
options: /win.publisherName", and dropping a required
`azureSignOptions` field fails with "must have required property
'certificateProfileName'" — a class the previous test could never detect.
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8979a443-6a28-4ae1-a2c9-b2b7d748e7e6

📥 Commits

Reviewing files that changed from the base of the PR and between 78462b0 and 141db8d.

📒 Files selected for processing (1)
  • apps/desktop/tests/package-win.spec.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The Windows packaging tests now load Electron Builder’s schema, reconstruct merged configurations from command-line arguments, validate them with Ajv, and cover base, Azure signing, certificate signing, and removed-option scenarios.

Changes

Windows configuration validation

Layer / File(s) Summary
Schema loading and configuration validation
apps/desktop/tests/package-win.spec.ts
The tests load the Electron Builder schema and use Ajv to validate complete reconstructed configurations. Validation errors distinguish unknown options from other schema failures.
Packaging scenario coverage
apps/desktop/tests/package-win.spec.ts
The tests validate the base configuration, Azure signing configuration, certificate signing configuration, and rejection of win.publisherName.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 141db

This PR strengthens release-configuration tests by validating generated arguments against the installed Electron Builder schema; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title uses a valid conventional-commit prefix and imperative wording, but it exceeds the 72-character limit at 77 characters. Shorten the title to 72 characters or fewer while preserving its description of schema validation tests.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the problem, implementation, validation results, and checklist; it does not provide a direct related-issue link.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Aug 23, 2026

Copy link
Copy Markdown
pnpm dlx https://pkg.pr.new/@pymodel/pythinker-code@141db8d
npx https://pkg.pr.new/@pymodel/pythinker-code@141db8d

commit: 141db8d

@elkaix
elkaix merged commit b943735 into main Aug 23, 2026
25 checks passed
@elkaix
elkaix deleted the test/eb-config-schema-validation branch August 23, 2026 16:13
elkaix added a commit that referenced this pull request Aug 23, 2026
## Related Issue

No issue — third and last hardening item from the `desktop-v0.2.1`
release failure (#167, #168).

## Problem

`Desktop Release` already supports `workflow_dispatch`, and both
platforms already build with `--publish never`, so a manual run is safe.
But every signing gate is tag-gated:

- `Require signing for tagged releases` (mac)
- `Verify macOS signatures and notarization`
- `Require signing for tagged Windows releases`
- `Verify Windows release signatures`

So a manual run skipped precisely the steps worth rehearsing. Those
checks first executed on the tag that depended on them — which is how
`desktop-v0.2.1` burned two builds: an electron-builder option removed
in v26, and a `stapler validate` aimed at the `.dmg` instead of the
`.app`. Both were findable in a rehearsal; there was no rehearsal that
could find them.

## What changed

The four verification steps lose their `if:`. Everything touching the
releases repository stays tag-gated:

| Stays tag-gated | Now runs on a rehearsal |
|---|---|
| `Require the tagged commit to be on main` | `Require signing` (mac +
win) |
| `Mint releases-repo token` (×3) | `Verify macOS signatures and
notarization` |
| `Create the draft release unless it already exists` | `Verify Windows
release signatures` |
| `Upload verified {macOS,Windows} release assets` | |
| `publish` job | |

A manual run is now the whole release path minus publication, with
installers attached to the run. Run it before any `desktop-v*` tag:

```
gh workflow run "Desktop Release" --ref main
```

It also closes the last untested corner: `packageManagerInvocation` adds
literal quotes on Windows for values containing spaces, and until now
only a tag build ever exercised that with a real publisher name.

## Verification

A test pins the arrangement, since re-adding one `if:` would restore the
hole without breaking anything visible.

| | |
|---|---|
| Restore the tag gate on the Windows credential check | `× runs every
signing gate on a manual rehearsal, not only on a tag` |
| Restored | 9 passed |

- `pnpm exec vitest run` (apps/desktop) — 155 passed, 14 files
- `pnpm run typecheck` — clean, both tsconfigs
- `oxlint --type-aware`, `check-no-comments.mjs` — clean

## Checklist

- [x] I have read the CONTRIBUTING document.
- [ ] I have linked a related issue — none; follow-up to #167 / #168.
- [x] I have added tests that prove my feature works.
- [x] Ran `gen-changesets` skill, or this PR needs no changeset — no
changeset: CI-only, nothing users can perceive.
- [x] Ran `gen-docs` skill, or this PR needs no doc update.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Release Process**
* Manual release rehearsals now run macOS and Windows signing and
signature verification without publishing releases.
* Release creation, asset uploads, and publishing remain limited to
tagged releases.

* **Tests**
* Updated workflow coverage to ensure signing and signature verification
always run during rehearsals, without conditional gating.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant