Update spectron app handler to pick VSCode window #744
Conversation
| const title = await this.webclient.getTitle(); | ||
|
|
||
| if ( | ||
| process.platform === 'win32' && |
There was a problem hiding this comment.
NodeJs always returns win32 regardless of which windows version is being used (https://nodejs.org/docs/latest-v8.x/api/process.html#process_process_platform).
| if ( | ||
| process.platform === 'win32' && | ||
| title !== '' && | ||
| /Visual Studio Code/.test(title) |
There was a problem hiding this comment.
We could do without this check but wanted to make sure we look for the vscode window in case the empty terminals start returning a title (currently they return an empty title).
Codecov Report
@@ Coverage Diff @@
## develop #744 +/- ##
========================================
Coverage 74.23% 74.23%
========================================
Files 163 163
Lines 6609 6609
Branches 1039 1039
========================================
Hits 4906 4906
Misses 1433 1433
Partials 270 270Continue to review full report at Codecov.
|
| for (let i = 0; i < count; i++) { | ||
| await this.webclient.windowByIndex(i); | ||
|
|
||
| if (/bootstrap\/index\.html/.test(await this.webclient.getUrl())) { |
There was a problem hiding this comment.
This could be a basic question, but why is the window that we want in focus during the Spectron test going to be named "...bootstrap/index.html"? Is that a Spectron convention?
There was a problem hiding this comment.
That has to do with how spectron loads the vscode window, it's normally a file path that represents the local vscode location being used.
I think the path might've changed with vscode moving to a newer electron version (now we get something like electron-browser/workbench/workbench.html) but I was only able to test in Windows and Mac OS so I kept this for now.
lcampos
force-pushed
the
lcampos/spectron-win-empty-terminal
branch
from
eb7617c to
14f7ecb
Compare
What does this PR do?
Updates our Spectron application handler and re-enables system tests in appveyor. The changes address a Spectron Windows OS issue (electron-userland/spectron#60) where multiple empty terminals get opened when running Spectron, causing our tests to fail in appveyor.
What issues does this PR fix or reference?
@W-5511648@