Skip to content

Set-up to run tests with vitest browser mode #1947

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

Merged
merged 2 commits into from
Apr 23, 2025
Merged

Set-up to run tests with vitest browser mode #1947

merged 2 commits into from
Apr 23, 2025

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented Apr 14, 2025

Depends on hypothesis/frontend-build#722

This is a POC which aims to replace karma as the test runner with vitest, using browser mode + playwright.

Karma has been deprecated and is no longer actively maintained, so we need to eventually find a replacement that allows us to continue writing our tests in the same way, and running a pre-bundled tests bundle.

This PR:

  • Adds vitest and playwright as dev dependencies.
  • Defines a new gulp task that bundles our tests, but does not run karma afterwards.
  • Adds Vitest configuration, enabling browser mode in headless chromium.
  • Adds a new npm task that runs tests with vitest rather than karma.

Todo

  • Fix couple of still failing tests. (Improve tests to work with both karma and vitest #1950)
  • Ensure code coverage reports are generated.
  • Fix code coverage error unless yarn.lock is deleted and dependencies are cleanly installed.
    • I did not push the changes, but in order to avoid a weird error, I had to delete yarn.lock and install dependencies again. Then coverage would be generated properly.
      I need to find what's the exact dependency conflict that causes this. My theory is that there are a bunch of instanbul instances that are conflicting, and may be fixed by simply uninstalling all karma stuff, including its istanbul plugin.
      EDIT: I just verified running yarn dedupe also fixes this issue. I assume it's because there are multiple instances of istanbul in probably incompatible versions, because multiple dependencies depend on it themeselves.
  • Install playwright dependencies during CI.
  • Document how to get everything set-up and automate where possible.
    • Makefile has been updated, so that we run yarn playwright install every time we run yarn install. That should keep playwright binaries up to date.
      Running that command when no updates are needed has a very low impact compared to the overall execution time of yarn install.
  • Support running tests with --grep <some_pattern> (Add support to run tests with vitest frontend-build#722)
  • Support live mode when --live is passed. (Add support to run tests with vitest frontend-build#722)
  • Remove karma and mocha once build is green.

Copy link

codecov bot commented Apr 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (e74fcd2) to head (0e6f9c2).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1947   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           70        70           
  Lines         1316      1316           
  Branches       486       486           
=========================================
  Hits          1316      1316           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@acelaya acelaya force-pushed the vitest-tests-poc branch 2 times, most recently from 7c733ce to 6850f6f Compare April 15, 2025 13:14
console.error(pendingErrorNotice);
throw pendingError;
}
});
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 throws a lo of false positives when used with vitest, and I reckon it is no longer needed with karma either, so I'm removing it.

@@ -12,31 +12,10 @@ sinon.assert.expose(assert, { prefix: null });
// karma-sinon plugins
globalThis.assert = assert;
globalThis.sinon = sinon;
globalThis.context = globalThis.context ?? globalThis.describe;
Copy link
Contributor Author

@acelaya acelaya Apr 15, 2025

Choose a reason for hiding this comment

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

Vitest does not have a global context, so this simply creates an alias from describe, which is basically what mocha does.

@acelaya acelaya force-pushed the vitest-tests-poc branch 3 times, most recently from f9059fd to 20fac69 Compare April 16, 2025 14:51
@acelaya acelaya force-pushed the vitest-tests-poc branch 2 times, most recently from ad961d9 to 2e57c32 Compare April 17, 2025 08:08
@acelaya acelaya force-pushed the vitest-tests-poc branch 2 times, most recently from b1b9cf7 to ffac4ac Compare April 17, 2025 08:52
@acelaya acelaya force-pushed the vitest-tests-poc branch 3 times, most recently from 721cdbf to 5cdb120 Compare April 18, 2025 08:21
yarn.lock Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most removals here are due to the execution of yarn dedupe, which was needed to solve a weird error when trying to generate code coverage reports.

By googling the error I found many people recommending the removal of the yarn.lock file, which did work, but introduced many more changes.

Based on that I assumed the issue was in the fact that istanbul was a dependency of multiple other dependencies, which was causing different conflicting versions to be installed.

Makefile Outdated
@@ -65,4 +65,5 @@ build: node_modules/.uptodate

node_modules/.uptodate: package.json yarn.lock
yarn install
yarn playwright install
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 has a low impact when the binaries are already up to date. In my machine it's around 0.6s, which is acceptable if we only run it when yarn install is also needed.

@acelaya acelaya requested a review from robertknight April 18, 2025 08:53
@acelaya acelaya marked this pull request as ready for review April 18, 2025 08:53
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Karma config file used to be in the src dir, but I have put this in the root dir. If there was a good reason for the old location, I think it should be possible to move this there.

@acelaya acelaya force-pushed the vitest-tests-poc branch 2 times, most recently from 555afbd to 1cfeef0 Compare April 22, 2025 10:19
@robertknight
Copy link
Member

I see a message about RadioGroup appear in the output, although it doesn't cause test failures:

[I]  ~/h/frontend-shared > make test
yarn test
[12:09:19] Using gulpfile ~/hypothesis/frontend-shared/gulpfile.js
[12:09:19] Starting 'test'...
[12:09:19] Starting 'build-test-css'...
[12:09:19] Starting '<anonymous>'...
[12:09:19] Building test bundle... (63 files)
[12:09:19] Finished 'build-test-css' after 366 ms
[12:09:22] Starting vitest...

 RUN  v3.1.1 /Users/robert/hypothesis/frontend-shared
      Coverage enabled with istanbul

stderr | build/scripts/tests.bundle.js > RadioGroup > when RadioGroup.Radio is used outside of RadioGroup > throws an error
Add @babel/plugin-transform-react-jsx-source to get a more detailed component stack. Note that you should not add it to production builds of your App for bundle size reasons.
 ✓  chromium  build/scripts/tests.bundle.js (651 tests) 908ms
...

Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

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

The playwright install command can be optimized by installing only the browsers we are going to use, especially in CI. Otherwise this looks good.

@acelaya acelaya requested a review from robertknight April 23, 2025 11:59
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