-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use WebdriverIOs driver setup mechanism #94
Conversation
If I set up a new project via
|
Co-authored-by: Sean Poulter <sean.poulter+gh@gmail.com>
@christian-bromann version |
I haven't been able to make progress on this, if anyone wants to push it over the goal line I would appreciate it. |
You've got our eyes on it @christian-bromann. How can we help? I'd suggest splitting the changes to the docs/imports out into a small quick PR, and we can focus on the behaviour changes separately. |
I hope to be able to take a look at this sometime this week. If you like to take this one feel free. |
|
||
#### `cachePath` | ||
|
||
Define a cache path to avoid re-downloading all bundles. This is useful for CI/CD to avoid re-downloading VSCode and Chromedriver for every testrun. | ||
Define a cache path to avoid re-downloading VS Code bundles. This is useful for CI/CD to avoid re-downloading VSCode and Chromedriver for every test run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is out of scope of this PR but would we want to try to normalize how we write VS Code/VSCode, and Chromedriver/ChromeDriver?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
"yargs-parser": "^21.1.1" | ||
}, | ||
"peerDependencies": { | ||
"chromedriver": "latest", | ||
"webdriverio": "^8.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand the blog post correctly, will this need to explicitly be ^8.14.0?
@@ -147,77 +137,24 @@ export default class VSCodeServiceLauncher extends ChromedriverServiceLauncher { | |||
throw new Error(`No key "${VSCODE_CAPABILITY_KEY}" found in caps`) | |||
} | |||
|
|||
if (versionsFileExist) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you want to keep versions.txt at all @christian-bromann?
This looks really close. I think I've got it working/failing to start locally. That's a win because ChromeDriver on Linux on ARM isn't officially supported or downloaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you can confirm how to handle the cache, and that you wanted to set the browserName = 'chrome' and
browserVersion` to the version of Chromium bundled with VS Code, then I think I've got most of it done.
The pipeline still fails but I suspect it's because the code building the download links from Puppeteer only supports a 3 digit version number, not a 4 digit like we extract from the manifest. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks for looking into this. Mind raising a new PR and we take it from there?
Caching for Chromedriver is handled by WebdriverIO. It doesn't cache the VS Code binary though which I am not sure will become a problem. I've implemented this with the idea in mind that test should work no matter if the user has an internet connection or not. That said, I am ok with moving forward without that feature and raise an issue to re-add it back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll open a PR when I find a moment.
I believe we could keep with your original intent with the cache if we pivot to store the chromium version instead of chromium and let WebdriverIO handle Chromium/ChromeDriver.
Since I haven't got VS Code to open yet, would you happen to know if we'll want to use ChromeDriver or can we use Chrome for Testing to interact with VS Code? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chrome for Testing and Chromedriver are different things. The first is the browser the other the driver. For automating VS Code we need Chromedriver and download the desired VS Code version. VS Code is an Electron application and "acts" as a browser here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like I don't have to touch the browserName
anyways. 🎉
a6c2fd4
to
4665af8
Compare
Closing in favor of #105 ... thanks @seanpoulter 🙌 |
This patch will remove
chromedriver
andwdio-chromedriver-service
in favor of WebdriverIOs own driver setup mechanism.Note: this currently doesn't work yet as Chrome for testing started to release Chromedriver starting v115. VS Code however currently still relies on v114 for their latest VS Code release.