-
-
Notifications
You must be signed in to change notification settings - Fork 304
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
feat(test-runner-saucelabs): add support for Sauce Connect 5 #2876
feat(test-runner-saucelabs): add support for Sauce Connect 5 #2876
Conversation
🦋 Changeset detectedLatest commit: 158b667 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
if (finalConnectOptions?.tunnelIdentifier) { | ||
console.warn('The `tunnelIdentifier` option is deprecated. Use `tunnelName` instead.'); | ||
finalConnectOptions.tunnelName = finalConnectOptions.tunnelIdentifier; | ||
delete finalConnectOptions.tunnelIdentifier; | ||
} |
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 option was an obvious change and it was easy to provide a migration path for it. Up to you if you want to include it.
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'd rather not, such changes are expected in minor version in 0.x.x range, and I think we are just a proxy to SauceLabs libs, so we don't need to do more that they do, if they remove it, we should follow and not make own deprecation policy.
Should you fix this, I'll be happy to merge and release this update, good work!
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.
Alright, I removed it 🙂
@@ -53,7 +53,7 @@ export class SauceLabsLauncherManager { | |||
this.connectionPromise = withTimeout( | |||
this.api.startSauceConnect({ | |||
...this.connectOptions, | |||
noSslBumpDomains: `127.0.0.1,localhost,${internalIp.v4.sync()}`, | |||
tlsPassthroughDomains: `^(127\\.0\\.0\\.1|localhost|${internalIp.v4.sync()?.replace(/\./g, '\\.')})$`, |
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.
All domains now must be passed a regex instead of a glob.
What I did
saucelabs
to9.0.0
to make use of Sauce Connect 5Notes
For some background,
test-runner-saucelabs
currently usessaucelabs@^7.2.0
, which downloads Sauce Connect version 4. That version has already reached EOL and starting May 5, 2025 SauceLabs will stop accepting traffic from that version. That means that at that pointtest-runner-saucelabs
would stop working as well.SauceLabs users need to upgrade to Sauce Connect 5 instead. To support that SauceLabs have now released
[email protected]
, which downloads and runs the newer version. That version contains breaking changes in terms of API and configuration options. That means that this version bump is a breaking change fortest-runner-saucelabs
users as well, as they might use options that have been changed.Bumping the version now would give
test-runner-saucelabs
users ample time to upgrade and adapt their projects before this package stops working completely beginning of May. SauceLabs has published a migration guide that helps with upgrading projects.With the changes in this PR I was able to connect to SauceLabs and run the tests in
test-runner-saucelabs
as well as in https://github.com/vaadin/web-components. I have only marked this a a minor change in the changeset, as the package has not had a major release yet.