Skip to content

Conversation

@andersk
Copy link
Contributor

@andersk andersk commented Mar 1, 2022

A big section of the test suite has actually been testing the builtin electron.remote rather than this module. Correcting that reveals two bugs.

This an equivalent of electron/electron#20925.
It’s needed for us to pass the test suite when we’re actually testing
this module instead of Electron’s builtin remote.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Semicolons are optional in JavaScript, except when they aren’t.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk andersk force-pushed the test-this-module branch from ce352d0 to dca79c2 Compare March 1, 2022 05:00
@andersk
Copy link
Contributor Author

andersk commented Mar 1, 2022

Oh. The second bug was just a missing semicolon issue from when the test was ported in 972b076.

With that fixed, everything passes.

@andersk andersk marked this pull request as ready for review March 1, 2022 05:02
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk andersk force-pushed the test-this-module branch from dca79c2 to 6dfa6f0 Compare March 1, 2022 05:36
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk andersk changed the title test: Test this module, not the builtin electron.remote Test this module, not the builtin electron.remote Mar 1, 2022
@andersk andersk changed the title Test this module, not the builtin electron.remote Test this module, not the builtin electron.remote; test with Electron 14, 15, 16, and 17 Mar 1, 2022
@andersk
Copy link
Contributor Author

andersk commented Mar 1, 2022

With a few other fixes, the test suite now passes with Electron 14, 15, 16, and 17 (except the error cause test, which I’ve conditionally disabled), so we can add those to CI.

In combination with #111, this will let us catch future index.d.ts typing errors in CI across all supported Electron versions.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@nornagon nornagon merged commit 8534ef0 into electron:main Mar 15, 2022
@nornagon
Copy link
Contributor

Thanks!

@electron-bot
Copy link

🎉 This PR is included in version 2.0.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

@andersk andersk deleted the test-this-module branch December 5, 2022 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants