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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: overhaul build with vite & playwright #2825

Merged
merged 2 commits into from Nov 16, 2023
Merged

build: overhaul build with vite & playwright #2825

merged 2 commits into from Nov 16, 2023

Conversation

hugo-vrijswijk
Copy link
Member

@hugo-vrijswijk hugo-vrijswijk commented Nov 6, 2023

Overhauls the build (and tests) by setting up Vite to build all projects, and playwright for the elements integration tests.

  • Build on each project is now done with Vite. This means both CommonJS and ESM are published!
  • Unit tests on elements use vitest (with browser mode). I couldn't get Vite to work with karma. Unfortunately, this also means no Stryker as it doesn't support browser mode (yet?). Browser setup uses playwright!
  • Integration tests on elements are done with playwright. The page objects had to be overhauled a little bit. But they are much faster (10 seconds vs 2.5 minutes on my machine) and a lot less flaky.
    • This also means no more dependency on chromedriver, and thus no dependency on the version of chrome that GHA has 馃コ
    • Playwright works so much nicer with web components, no more crazy helper functions for passing shadow root are needed. And many assertions are already configured to wait and retry
    • Not all assertions are rewritten to use playwrights expect (which retries and is less flaky). But that could be done in the future
  • Typescript is used for typechecking the project only, as vite handles building
  • All packages pass publint and arethetypeswrong
  • Dependencies of building each project should all work when using nx (build on one project depends on the other)
  • Vite and playwright maybe could've been separate PR's, but a lot of changes from Vite required changes in the integration tests, so it would be a lot of work to separate them now
  • No more slow webpack 馃コ
  • Only elements unit tests are done with vitest. Others still use mocha. I'm open to changing it, it would make the test and build setup the same (though they are relatively simple now)
  • Sorry for the giant diff 馃檲

BREAKING CHANGE: mutation-testing-report-schema and mutation-testing-metrics are now ESM
BREAKING CHANGE: report-schema MutantStatus is a union type instead of TS enum

@nicojs
Copy link
Member

nicojs commented Nov 7, 2023

This improvement is freakin' amazing 馃ぉ! I haven't had time to look at it yet, but I will try to do it in a 1h train ride later today.

Copy link
Member

@nicojs nicojs left a comment

Choose a reason for hiding this comment

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

We've looked through the code together. 馃 Some findings:

  • Vite is now responsible for bundling each project. Only elements should be bundled. We should use plain tsc --build like before.
  • It's OK to have a breaking change to move to esm. I didn't even realize this project was not ESM yet.
  • Dev experience:
    • We should have an up-to-date CONTRIBUTING.md, including a cheat sheet.
    • Some commands should work from the root. Useful ones might be: test and build?
    • CTRL+SHIFT+B support and debugging (launch.json) should be updated

Copy link
Member

@nicojs nicojs left a comment

Choose a reason for hiding this comment

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

This looks a lot better :). Just a few remarks:

  • The task "Debug Unit Tests (elements)" is not working. How do we debug elements unit tests?
  • The breaking change of not exporting MutantStatus anymore will hurt a bit in StrykerJS, as we use it all over the place (it was the only way to work with the mutant result). But I see why it is necessary.
  • How does one debug the playwright tests? I see the playwright plugin is added to the recommended plugins, can I use that? Maybe we can add this to the CONTRIBUTING.md?

I've updated the schema2ts script, so pull before changing anything else.

hugo-vrijswijk and others added 2 commits November 16, 2023 13:50
BREAKING CHANGE: mutation-testing-report-schema and mutation-testing-metrics are now ESM
BREAKING CHANGE: report-schema `MutantStatus` is a union type instead of TS enum
@hugo-vrijswijk hugo-vrijswijk merged commit 3176cdc into master Nov 16, 2023
6 checks passed
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

2 participants