sinonjs / fake-timers

Fake setTimeout and friends (collectively known as "timers"). Useful in your JavaScript tests. Extracted from Sinon.JS
BSD 3-Clause "New" or "Revised" License
797 stars 104 forks source link

Safari test run breaks in Sauce Lab due to missing W3C WebDriver support #380

Closed fatso83 closed 8 months ago

fatso83 commented 3 years ago

Full background: https://github.com/sinonjs/sinon/issues/2372.

Basically, Safari tests breaks with v12 and up. This needs to be fixed in min-wd (mantoni/min-webdriver#31).

benjamingr commented 3 years ago

Would it make sense to just migrate to WebDriverIO?

@christian-bromann given the visibility and popularity of this project is this something you can assist with? (feel free to say "no", no intention to pressure)

Ideally someone looking to contribute more ( @itayperry ) could take this if you can mentor them on the migration.

christian-bromann commented 3 years ago

Would it make sense to just migrate to WebDriverIO?

I don't think so. WebdriverIO has no capabilities to run Mocha, Jasmine and Cucumber in a browser for unit test purposes. However given that many libraries still would like to do such things I will look into supporting this. Short term I believe fixing the protocol bug in min-webdriver makes the most sense IMO.

benjamingr commented 3 years ago

@christian-bromann thanks for the quick response it's appreciated! 🙏

The logic for running mocha tests in browsers isn't in min-webdriver it's in mochify. That could (at least optionally?) just use WebdriverIO right?

christian-bromann commented 3 years ago

Yes it could. Then easiest would be to implement a browserify plugin compatible interface into webdriverio similar as min-wd does it.

benjamingr commented 3 years ago

Great, @itayperry is this something you'd be interested in contributing? It could be a good chance to contribute to another project too (WebdriverIO)

itayperry commented 3 years ago

Hi @benjamingr :) Yes, I'd be happy - I'm still working on 375 and I also wanna contribute to 374. I've been slow lately, so if no one takes this one, I definitely will :) I don't wanna stall you.

itayperry commented 2 years ago

... to implement a browserify plugin compatible interface into webdriverio similar as min-wd does it

I'd be happy to do it - how / where can I get started? I never did this before but it sounds interesting! :)

mantoni commented 2 years ago

I'm actually working on a Mochify rewrite (https://github.com/mantoni/mochify.js has a rewrite branch), and the current state seems to work fine. I just don't have enough free time to get it across the finish line.

itayperry commented 2 years ago

I just don't have enough free time to get it across the finish line.

@mantoni anything I can help with? (with some guidance).

mantoni commented 2 years ago

@itayperry You could start by checking out this branch and see if you can run Safari tests for fake-timers with it: https://github.com/mantoni/mochify.js/pull/241

itayperry commented 2 years ago

Hi @mantoni, I saw that you just merged rewrite-tests to rewrite branch - which made it easier for me to check out! So thank you - I couldn't find a way to check out an unmerged PR code (I read a guide but had too many issues - https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally - nevermind).

I'll be honest, I don't really know how to activate it - I tried deleting mochify from fake-timers and then using npm-link to package-link my local mochify rewrite branch. I had a few errors and I seem to be stuck.

In fact, I even tried to npm-link the basic master branch of my local mochify clone just to see if things work normally - and that also didn't work, I think I'm doing something wrong.

One more question - How do get it running? I saw this node cli --reporter nyan but couldn't quite understand it.

mantoni commented 2 years ago

Thanks for looking into it. There is clearly not enough documentation yet. I managed to run tests by invoking the new Mochify CLI locally using a relative path.

However, I will make a 0.1.0 release this afternoon and send a PR here. We can then iterate on that together. It's a lot easier when we have a running example to talk about.

itayperry commented 2 years ago

Sounds great! I'll wait for the PR. Thank you for being patient and helpful about all of it.

mantoni commented 2 years ago

@itayperry It tried all evening, and it works for Safari, but not for any other browser 🙈. Your help on the above PR and / or the Mochify rewrite would be highly appreciated.

itayperry commented 2 years ago

@mantoni I'd be very happy to help! Where should I start? (I might ask for some advice/tips).

mantoni commented 2 years ago

I figured it out for this project and have a fix in https://github.com/mantoni/mochify.js/pull/244 (spoiler: there's a very subtle bug in the geckodriver).

I'm happy to provide assistance for anyone who is willing to help. I'm sorry for the lack of documentation. A good way to start is to read through the initial "Mochify Rewrite" issue: https://github.com/mantoni/mochify.js/issues/229

Issues for the rewite can be found here: https://github.com/mantoni/mochify.js/milestone/1

fatso83 commented 1 year ago

Getting this over the finish line is basically #395, for anyone interested.

fatso83 commented 1 year ago

WebDriverIO just merged the fix for "Allow to run unit tests within the browser #6928". Not sure if this makes any difference to this stale issue, but thought I would just mention it 😄

christian-bromann commented 1 year ago

Ha! Yeah, WebdriverIO now supports running tests in the browser. It would be ideal for sinonjs but the Mochify PR seems to be almost ready to ship I guess.

fatso83 commented 1 year ago

Yeah, all that PR needs to finish is someone that is not pre-occupied with feeding and putting kids to bed in their few allowable hours off 😄 Could perhaps put a 💵 bounty on it to get it proper tested and finished?

stale[bot] commented 8 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.