feat: adding electron support for cypress component testing #15731
Conversation
JessicaSachs
requested review from
chrisbreiding and
kuceb
and removed request for
a team
|
Thanks for taking the time to open a PR!
|
JessicaSachs
force-pushed
the
feat/electron-support-for-component-testing
branch
from
bb4453a to
dc37ed8
Compare
flotwig
force-pushed
the
electron-code-1-signal
branch
from
a1acd80 to
979babe
Compare
| // instance.once('exit', () => { | ||
| // options.onBrowserClose() | ||
|
|
||
| return cleanup() | ||
| }) | ||
| // return cleanup() | ||
| // }) |
There was a problem hiding this comment.
Is this right? This seems important
| if (!instance) { | ||
| return Promise.resolve() | ||
| } | ||
|
|
||
| return new Promise((resolve) => { | ||
| if (unbind) { | ||
| instance.removeAllListeners() | ||
|
|
||
| return resolve.apply(null, [unbind]) |
There was a problem hiding this comment.
trying to understand this change...
unbind is true only in browsers.open:
cypress/packages/server/lib/browsers/index.js
Lines 179 to 180 in 1282c70
the resolved value appears to be irrelevant, so you could just resolve()
so the new logic is that when a browser is opened and there is an instance, instead of killing the existing browser instance, just remove all listeners from it and resolve kill?
that seems like, wrong to me... instance.kill() never even runs with this patch, i feel like that's bound to cause issues somewhere - maybe in cypress open?
and cleanup is never called with this patch, so instance will still exist
JessicaSachs
force-pushed
the
feat/electron-support-for-component-testing
branch
from
2ce85e9 to
3447f2f
Compare
JessicaSachs
force-pushed
the
feat/electron-support-for-component-testing
branch
from
3447f2f to
1701807
Compare
…com:cypress-io/cypress into feat/electron-support-for-component-testing
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
User facing changelog
By default the Component Test Runner will launch in Electron to improve UX and maintain consistency with the E2E Runner.
Additional details
We did not support Electron in the Component Test Runner. This means that users would have needed to install Chrome on their CI machines, which isn't acceptable. Now, we support Electron, Chrome, and Firefox.
Additionally, we use Electron by default. This is a UX improvement so that there is only on system tray application.
Adding Electron support for Cypress CT revealed an issue with our logic when we destroy Electron BrowserWindows. We attempt to do so safely, but our safety checks don't prevent certain race conditions. I tried to refactor server/lib/browsers.js to make sure the instance wasn't cached so we couldn't destroy it twice, but this led to other test failures.
Rather than risk deeper breaking changes, we limited the fix so that we only tear down the browser when we're not in Electron. This behaves as expected.
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?cypress.schema.json?