Skip to content

fix: test idempotency issues#51504

Merged
ckerr merged 7 commits into
mainfrom
fix/test-idempotency-issues
May 6, 2026
Merged

fix: test idempotency issues#51504
ckerr merged 7 commits into
mainfrom
fix/test-idempotency-issues

Conversation

@ckerr

@ckerr ckerr commented May 5, 2026

Copy link
Copy Markdown
Member

Description of Change

Fix some test idempotency issues identified by Claude.

Checklist

Release Notes

Notes: none.

@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label May 5, 2026
@ckerr ckerr marked this pull request as draft May 5, 2026 16:33
@ckerr ckerr added semver/patch backwards-compatible bug fixes target/40-x-y PR should also be added to the "40-x-y" branch. target/41-x-y PR should also be added to the "41-x-y" branch. target/42-x-y PR should also be added to the "42-x-y" branch. labels May 5, 2026
@ckerr ckerr marked this pull request as ready for review May 5, 2026 18:46
ckerr added 7 commits May 5, 2026 14:42
The getPath(name) and setPath(name, path) describe blocks both call
app.setPath('music', ...) without restoring the original value. This
causes subsequent tests that call app.getPath('music') to get a
corrupted path if test ordering changes.

Capture the original music path and restore it in afterEach().
The commandLine.hasSwitch and commandLine.getSwitchValue tests append
switches (foobar1, foobar) that are never removed. These persist for
the entire process lifetime, which could affect tests that check for
switch presence or absence if test ordering changes.

Add afterEach hooks that call removeSwitch for each appended switch.
The 'validates process APIs access in sandboxed renderer' test sets
process.env.sandboxmain = 'foo' without cleaning it up. This env var
persists for all subsequent tests and could affect tests that compare
process.env snapshots.

Use defer() to delete the env var after the test completes.
The release-notes spec sets NOTES_CACHE_PATH in before() but never
deletes it. This env var persists for all subsequent test files and
could interfere with any code that checks for its presence.

Add an after() hook to delete the env var when the suite completes.
The app name tests restore app.name at the end of each test body. If
an assertion fails before the restoration line, the name stays
corrupted for all subsequent tests.

Move restoration to an afterEach hook so it always runs, and remove
the now-redundant inline restoration calls.
The 'can download from custom protocols' test registers an HTTP
protocol handler on session.defaultSession but never unregisters it.
This handler persists globally and could interfere with other tests
that use the same protocol name or the default session.

Use defer() to ensure the protocol is unregistered after the test.
The 'reporting api' test sets setCertificateVerifyProc on
defaultSession to accept all certificates, but the finally block
only destroys the window and closes the server. The permissive
certificate handler persists globally, causing all subsequent tests
using defaultSession HTTPS requests to skip certificate verification.

Reset the handler to null in the finally block.
@ckerr ckerr force-pushed the fix/test-idempotency-issues branch from 52a6de1 to 3f7f7a1 Compare May 5, 2026 19:50
@github-actions github-actions Bot added the target/43-x-y PR should also be added to the "43-x-y" branch. label May 6, 2026
@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label May 6, 2026
@ckerr ckerr merged commit dd4bb88 into main May 6, 2026
118 of 121 checks passed
@release-clerk

release-clerk Bot commented May 6, 2026

Copy link
Copy Markdown

No Release Notes

@trop

trop Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "42-x-y", please check out #51526

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/patch backwards-compatible bug fixes target/40-x-y PR should also be added to the "40-x-y" branch. target/41-x-y PR should also be added to the "41-x-y" branch. target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants