Skip to content
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

Bump saucelabs dep to 6.2.2 #256

Merged
merged 1 commit into from Aug 26, 2022

Conversation

taymoork2
Copy link
Contributor

@taymoork2 taymoork2 commented Oct 25, 2021

Unsure if this will fix instability issues we're seeing on our end, but at least this will get the SC Tunnel to the latest version
Not seeing any breaking changes that need to addressed from bumping, unless we bump to v7 where node v10 support will be dropped

@seanpoulter
Copy link

I'm also here investigating instability issues. Would folks be willing to cut a new release with saucelabs@6.2.2 and a major version that drops support for Node.js v10 and uses saucelabs@7.1.3? Let me know, I'm willing to drive it forward.

@seanpoulter
Copy link

With package.json:

  "devDependencies": {
    …
    "saucelabs": "^7.1.3",
    …
  },
  "overrides": {
    "karma-sauce-launcher": {
      "saucelabs": "$saucelabs"
    }
  },

Then saucelabs@7.1.3 seems to worked this far:

image

@Havunen
Copy link

Havunen commented May 27, 2022

Who is maintaining this package, it would be good to get saucelabs updated and new version of karma-sauce-launcher published to NPM @vojtajina @rcebulko @dignifiedquire @johnjbarton

Comment on lines +90 to +91
'log.json',
'old-log.json'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What motivated this change? The rest of the changes appear to be formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a result of going from v4 to v6, it switched from an Array of Objects to an Array of strings
I believe it fails otherwise

https://github.com/saucelabs/node-saucelabs/blob/main/README.md?plain=1#L108=

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think i just ran prettier through it hence the formatting changes

@lo1tuma
Copy link

lo1tuma commented Jul 20, 2022

Is there anything I can help with, to get this change merged?

@seanpoulter
Copy link

I'm available to help drive this forwards but I'm not a maintainer. If it was up to me as a reviewer I'd suggest we split the formatting from the dependency update so it is very low effort to approve.

I'd also suggest that we cut a release for saucelabs v6.x and v7.x. I'm happy to make those PRs today if anyone's available. 🙏

@lo1tuma
Copy link

lo1tuma commented Jul 20, 2022

@jginsburgn would you be able to help us with this topic?

@lo1tuma
Copy link

lo1tuma commented Jul 21, 2022

ping @wswebcreation

@Havunen
Copy link

Havunen commented Jul 21, 2022

It would make more sense to bump saucelabs to latest version IMO

@taymoork2
Copy link
Contributor Author

It would make more sense to bump saucelabs to latest version IMO

@Havunen the reason I didn't bump to v7 was because it dropped support for node v10 and that's technically a breaking change, which is on the maintainers to address

@taymoork2
Copy link
Contributor Author

@lo1tuma if you need a slightly more modern solution, it might be best to switch to @web/test-runner or just use builtin-browsers on your CI images
We (at Webex) tried @web/test-runner, however both sauce and browserstack plugins put the tests in an iframe and that wasnt working with WebRTC related tests, so we switched to the builtin browsers provided by CircleCI's node image

The alternative to all that is to use the overrides property in the newer version of npm or resolutions in yarn, as mentioned above

@taymoork2 taymoork2 changed the title Bump saucelabs dep to 6.2.1 Bump saucelabs dep to 6.2.2 Jul 25, 2022
@jginsburgn
Copy link
Member

@jginsburgn would you be able to help us with this topic?

Sorry for my delay in answering. I am no longer a good primary PoC for the Karma projects. It's better to tag @karma-runner/google-web-test-team directly!

@taymoork2
Copy link
Contributor Author

@wswebcreation can we can get your review on this?

@Havunen
Copy link

Havunen commented Aug 10, 2022

Maybe we should create a new launcher with latest version of saucelabs setup? This project seems abandoned

@seanpoulter
Copy link

Who do we talk to so we can check if we can get a fork in saucelabs/ instead of karma-runner/?

@barrtender
Copy link

Hey everyone, sorry for the slow reply here.

I'm on that @karma-runner/google-web-test-team that @jginsburgn mentioned and have been trying to ramp up on the various Karma plugins that we own and what their maintenance plans are.

Unfortunately for plugins like this where we don't use them internally we aren't nearly as plugged in to the changes that need to be made to keep them up to date. It would be best if someone that actually used it would be able to take ownership of the plugin so that it can get the care it deserves.

If someone would like to volunteer for that I can change the README in this repo to point to your fork. This is a project that is licensed with the MIT license so forking is totally approved.

Let me know if there's any other questions. I'm fairly new on the team so like I said I'm still ramping up on things but if I can't answer them myself I should be able to find someone that can.

Thanks, and sorry again for the slow reply.
@barrtender

@taymoork2
Copy link
Contributor Author

@barrtender thanks for the reply

I think the only maintenace that needs to be done on this repo is just making the sure the saucelabs dependency is up to date. So maintenace should be relatively low effort in that regard, and I'm guessing dependabot should be able to handle it going forward.
The only thing maintainers would need to address when upgrading Saucelabs to the latest version, it drops support for Node v10, so a Breaking Change will need to be addressed
If you'd still want the plugin forked, I'd suggest having someone from Saucelabs take over maintance.

@wswebcreation wswebcreation merged commit a679362 into karma-runner:master Aug 26, 2022
@wswebcreation
Copy link
Collaborator

Merged it, thanks! Now need to see how to release a new version

@wswebcreation
Copy link
Collaborator

Hi @barrtender

I'm trying to release a new version but this was set up by @rcebulko in the past. It would do an automatic release based on commit messages with the https://github.com/karmarunnerbot, see for example #243.
This is not working anymore. Can we have a conversation about how to resolve the releasing strategy for this plugin?

Grtz

Wim Selles @wswebcreation

@Havunen
Copy link

Havunen commented Aug 26, 2022

Thanks a lot for merging this, I'm looking forward for the new version!
Now that you are at it, is it possible to also release v7 saucelabs?

but at least this will get the SC Tunnel to the latest version
Not seeing any breaking changes that need to addressed from bumping, unless we bump to v7 where node v10 support will be dropped

If NodeJs support is seeing as an issue, then maybe this plugin could also make a major version bump to indicate the breaking change and old nodejs users can stick with old version.

@wswebcreation
Copy link
Collaborator

Hi @Havunen

thanks, but I can’t release, it seems that the pipeline is broken. I need help from the karma team to move this of Travis and run this from GHA

Then I can create a new release and in the end also release V4 which will drop support for NodeJs 10

@jginsburgn
Copy link
Member

Hi @Havunen

thanks, but I can’t release, it seems that the pipeline is broken. I need help from the karma team to move this of Travis and run this from GHA

Then I can create a new release and in the end also release V4 which will drop support for NodeJs 10

@wswebcreation if you (or anyone) are willing to create a PR like karma-runner/karma-chrome-launcher#249, I can take a look and approve to get GHA working and this fix released.

@taymoork2
Copy link
Contributor Author

taymoork2 commented Aug 27, 2022

@jginsburgn does it need to be exact copy of chrome-launcher or just converting the existing travis config to GHA?

@taymoork2
Copy link
Contributor Author

taymoork2 commented Aug 27, 2022

@jginsburgn this seems to be the bear minimum to convert the existing travis yaml to a github action

  1. Checkout code
  2. Install Node
  3. Install yarn
  4. install deps
  5. run samples command
  6. build
  7. semantic release

https://github.com/semantic-release/semantic-release/blob/master/docs/recipes/ci-configurations/github-actions.md#node-project-configuration

@wswebcreation
Copy link
Collaborator

@jginsburgn

Thanks, I can take a look at it this week

@wswebcreation
Copy link
Collaborator

@jginsburgn

I've created #274

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants