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

[joss][tests] Use playwright for testing multiple browsers #19

Merged
merged 3 commits into from
Mar 5, 2024

Conversation

sneakers-the-rat
Copy link

Part of: openjournals/joss-reviews#6336

  • I implemented tests for the changes I made OR
  • These changes do not require tests because these changes are adding test cases

Problem

Puppeteer is made for chromium-browsers only. Google is trying to own the web via chrome. Part of the problem is a constant shifting target that breaks other browsers. Ergo testing against other browsers is important for a free web.

This PR

  • Switch puppeteer with playwright - which is developed by the same team as puppeteer for testing across browsers
  • Adds setup and teardown for http server so tests run.

Split into two commits so it's easier to see changes - first commit is just substantive changes, and second commit is linting.

Really simple changes, just wrap tests in an additional forEach + describe block that tests with a given browser as described in the mocha docs. Use mocha global fixtures again as described in the docs

@katjaq
Copy link
Member

katjaq commented Mar 5, 2024

Thank you so much for this PR @sneakers-the-rat ! That's a fantastic upgrade ⭐

Also, thank you for all your other comments you had during the review process – we are working on them – and also for your linked discussion on mastodon! 😍 ❤️ So kind! We would be super happy about the PR you describe there. This is an amazing review process – we learn so much from you. Thank you from all 3 of us! <3

@katjaq katjaq merged commit a35dcc0 into neuroanatomy:master Mar 5, 2024
@sneakers-the-rat
Copy link
Author

Fabulous :) glad it's helpful. I'm not done with my review yet! just need to find time to finish it. obvi it looks good to me but i will at least show a demo of webpacking it into a single page and see how else i can be useful <3

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.

2 participants