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

migrate from karma to wtr #75

Merged
merged 2 commits into from
Feb 25, 2025

Conversation

keithamus
Copy link
Member

In #73 (and #74) I noticed the build is failing - it looks like something to do with our karma setup and how the github actions runner is using a newer version of ubuntu which has additional apparmor restrictions around Chrome. We could look into fixing that, but considering we've dropped Karma in favour of WTR in many many places (github/paste-markdown#97, https://github.com/github/axe-github/pull/106, github/filter-input-element#23, github/include-fragment-element#93, github/image-crop-element#46, github/clipboard-copy-element#67, github/auto-check-element#62, github/include-fragment-element#86, github/relative-time-element#178, github/template-parts#59, and so on) I figured the easier lift was to simply replace Karma with wtr and be done with it.

So this PR replaces karma, mocha and related plugins with wtr. It introduces a wtr config, which uses esbuild to convert the typescript into JS, and tests using Chromium. The config is relatively stock, and was copypasted from one of our custom element libraries. This also slightly refactors the test & pretest scripts; because we no longer need to run npm run build (thanks to the wtr esbuild plugin) we can drop that, and instead use pretest to run lint, leaving test to be an alias for wtr.

@Copilot Copilot bot review requested due to automatic review settings February 24, 2025 09:46
@keithamus keithamus requested a review from a team as a code owner February 24, 2025 09:46
@keithamus keithamus requested a review from jibrang February 24, 2025 09:46

Choose a reason for hiding this comment

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

PR Overview

This PR migrates the testing setup from Karma to Web Test Runner (wtr) by removing Karma-related configurations and updating test files to import source TypeScript modules instead of pre-built JavaScript files. Key changes include the addition of a new wtr configuration using esbuild, updating import paths in all test files, and completely removing the Karma configuration.

Reviewed Changes

File Description
web-test-runner.config.js Introduces a new wtr config using esbuild for TypeScript support
test/index.js Updates import paths to reference TypeScript source and adds chai
test/element-checkvisibility.js Updates import paths to reference TypeScript source and adds chai
test/promise-withResolvers.js Updates import paths to reference TypeScript source and adds chai
test/requestidlecallback.js Updates import paths to reference TypeScript source and adds chai
test/clipboarditem.js Updates import paths to reference TypeScript source and adds chai
test/navigator-clipboard.js Updates import paths to reference TypeScript source and adds chai
test/karma.config.cjs Removes the Karma configuration file as part of the migration

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (6)

test/index.js:2

  • Verify that the test runner configuration properly transpiles and resolves the imported TypeScript modules in a JavaScript test file. If not, consider adding a file extension alias or updating the configuration.
import {apply, isPolyfilled, isSupported} from '../src/index.ts'

test/element-checkvisibility.js:2

  • Confirm that the test runner is set up to correctly handle TypeScript imports in this JavaScript test file to avoid module resolution issues.
import {apply, isPolyfilled, isSupported, checkVisibility} from '../src/element-checkvisibility.ts'

test/promise-withResolvers.js:2

  • Ensure that the configuration for wtr properly transpiles TypeScript modules imported in JavaScript test files. Validate the module resolution for consistency.
import {apply, isPolyfilled, isSupported, withResolvers} from '../src/promise-withResolvers.ts'

test/requestidlecallback.js:2

  • Check that the TypeScript file import is correctly resolved by the test runner and transpiled as expected. Adjust configuration if necessary.
import {apply, isPolyfilled, isSupported, requestIdleCallback} from '../src/requestidlecallback.ts'

test/clipboarditem.js:2

  • Verify that the test runner handles TypeScript module imports correctly in this JavaScript test file, ensuring proper transpilation and resolution.
import {ClipboardItem, apply, isSupported, isPolyfilled} from '../src/clipboarditem.ts'

test/navigator-clipboard.js:2

  • Ensure that the transition from importing JavaScript modules to TypeScript modules in this test file is supported by the wtr configuration. Consider verifying module resolution if any issues arise.
import {clipboardRead, clipboardWrite, apply, isPolyfilled, isSupported} from '../src/navigator-clipboard.ts'

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

@keithamus
Copy link
Member Author

To save you some clicks I'll paste the relevant part of the actions logs to demonstrate this is working:

> @github/[email protected] pretest
> npm run lint


> @github/[email protected] lint
> eslint . --ignore-path .gitignore

Browserslist: caniuse-lite is outdated. Please run:
  npx update-browserslist-db@latest
  Why you should do it regularly: https://github.com/browserslist/update-db#readme

> @github/[email protected] test
> wtr


Chrome: |██████████                    | 0/6 test files | 0 passed, 0 failed

Running tests...

Running 6 test files...


Chrome: |██████████████████████████████| 6/6 test files | 14 passed, 0 failed

Finished running tests in 8.3s, all tests passed! 🎉

Copy link
Contributor

@dgreif dgreif left a comment

Choose a reason for hiding this comment

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

One small thing for CI, but the rest of the changes look good. I love how easy of a swap that was!

@keithamus keithamus enabled auto-merge February 25, 2025 15:37
@keithamus keithamus merged commit 1dd097d into github:main Feb 25, 2025
1 check passed
This was referenced Feb 25, 2025
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