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

Browser-based tests should be run under continuous integration #7812

Closed
Jermolene opened this issue Oct 25, 2023 · 12 comments · Fixed by #7820
Closed

Browser-based tests should be run under continuous integration #7812

Jermolene opened this issue Oct 25, 2023 · 12 comments · Fixed by #7820

Comments

@Jermolene
Copy link
Owner

Our test suite runs under both Node.js and in the browser. However, our continuous integration implementation only runs the Node.js tests. There have been cases where the browser-based tests have been broken without anybody being aware.

I would very much like some help to get the browser-based tests running under CI, using something like Puppeteer or Playwright. The browser-based tests should be run simultaneously with the regular Node.js-based tests. We will need to figure out a way to reach into the test wiki and extract the test results.

Would anyone be able to take on this task?

Questions and suggestions welcome.

@rmunn
Copy link
Contributor

rmunn commented Oct 27, 2023

I would definitely recommend Playwright from my own personal experience. I can't commit to taking on this task right now, but if nobody else picks it up in the next couple weeks, then around mid-November I might get enough free time to give this a go.

@Jermolene
Copy link
Owner Author

Jermolene commented Oct 27, 2023

Thanks @rmunn. I have made use of Playwright within GitHub Actions as part of the TWPub project and found it pretty straightforward. For CI I'd like to be able to use Playwright capability to test across multiple browsers.

@Jermolene
Copy link
Owner Author

(oops I mistakenly typed Puppeteer in the previous comment but meant Playwright)

@pmario
Copy link
Contributor

pmario commented Oct 27, 2023

@pmario
Copy link
Contributor

pmario commented Oct 27, 2023

also related to: #6845

@EvidentlyCube
Copy link
Contributor

Also related: https://talk.tiddlywiki.org/t/automated-tiddlywiki-testing-with-playwright/8299

If it's just a matter of using Playwright to run the currently existing UI tests and verifying there are no complaints I may setup a proof of concept next week.

@EvidentlyCube
Copy link
Contributor

I actually had some time today and used it to make the above PR as a starting point.

@Jermolene
Copy link
Owner Author

If it's just a matter of using Playwright to run the currently existing UI tests and verifying there are no complaints I may setup a proof of concept next week.
I actually had some time today and used it to make the above PR as a starting point.

Great, thank you @EvidentlyCube. I think getting the existing tests running is a great start, and will make it easier to add the end-to-end testing in due course.

@EvidentlyCube
Copy link
Contributor

@Jermolene I think it might be worth considering whether to keep the e2e tests in the same repository or a separate one. The benefits of keeping it all in the same repository are obvious, but there are still cons to that:

  • Right now TW has no dependencies. Playwright tests require at minimimum putting @playwright/test in dev dependencies since this library has the necessary test and expect function-objects for the tests to run. The runner can be invoked with npx playwright run without putting it into the deps though.
  • It's a really good idea to also have ESLint to detect missing awaits which can create race conditions that are difficult to spot and create confusing errors. That would be another dependency.

It really depends on how this dependency purity is important for TW, if it's okay to add those then I spend another moment on a further proof of concept for that.

@Jermolene
Copy link
Owner Author

Hi @EvidentlyCube I am OK with having modest dev dependencies; my concern is that installing dependencies is conceptually tricky for end users. We do already have ESLint as a dev dependency (although @linonetwo has raised the suggestion of using DPrint instead in #7474).

@pmario
Copy link
Contributor

pmario commented Oct 30, 2023

@Jermolene -- There is a PR: #7596

@Jermolene
Copy link
Owner Author

@Jermolene -- There is a PR: #7596

Thanks @pmario I had forgotten about that.

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 a pull request may close this issue.

4 participants