-
Notifications
You must be signed in to change notification settings - Fork 414
test(integration): begin moving from Karma to Web Test Runner #5386
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
Conversation
Turns out `import @web/test-runner-mocha` _seemed_ to expose the mocha globals, but they were actually still being replaced because the autorun script from the test framework has its own inlined copy of mocha. Also a lot of tests were failing because some kind of watcher registered by the test framework was getting confused by the setup stuff being run twice.
it's used in a few tests
will fix them later
way simpler to rollup the test file, same as karma does
by skipping all the broken ones
will switch everything over later
// Cache reused between each compilation to speed up the compilation time. | ||
let cache; | ||
|
||
export default async (ctx) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a custom wrapper of @lwc/rollup-plugin
. It's largely copied from the one used for Karma tests. The primary reason we have this seems to be so that we can use different configs based on the file path of tests.
I don't like that pattern, and eventually I want to investigate using a normal human config file or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are our custom matchers. The bulk of the file is just copied from the karma version. Hopefully can be simplified once they are actually implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from Karma and cleaned up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New setup specific to WTR, largely related to bridging the gaps between frameworks.
TEMPLATE_CLASS_NAME_OBJECT_BINDING, | ||
}; | ||
|
||
window.TestUtils = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes importing from test-utils
and sometimes accessing window.TestUtils
is a really annoying pattern. Eventually I want to clean up the tests and have everything import from ./helpers/utils.mjs
.
reaching out breaks wtr
the test files originally had changes that got reverted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. It queues up the remaining work pretty well and makes it so that the work can be split or handed off another engineer as needed.
- release | ||
- 'spring*' | ||
- 'summer*' | ||
- 'winter*' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be better to just run on master
at first, since older branches won't have the tweaks that are necessary for the Karma-to-WTR migration. We can still run Karma locally for back-ported bug fixes.
@@ -218,7 +218,7 @@ export default tseslint.config( | |||
}, | |||
}, | |||
{ | |||
files: ['packages/@lwc/integration-karma/**'], | |||
files: ['packages/@lwc/integration-karma/**', 'packages/@lwc/integration-not-karma/**'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent package name.
}); | ||
}; | ||
|
||
const defaultRollupPlugin = createRollupPlugin(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this is a bit weird. It's okay for now, but it seems like something that the rollup plugin should be able to handle for us, without creating three different versions of the plugin.
} | ||
}); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just making sure I understand: this is a placeholder, where actual implementations of the custom matchers will be implemented in future PRs? If so, 👍
iirc it's vite that broke them
Details
Karma has been deprecated for a long time, and the recent end of life for sauce v2 is forcing us to finally move to something else. We've decided to move to Web Test Runner. One of the key differences between the two is that Web Test Runner is based on modern ESM imports, so there's minimal bundling and maximal modularity, while our Karma tests are an absolute mess of adding wrappers, injecting globals, and bundling in far too many different ways. Untangling all of the Karma tests so that they work with Web Test Runner is a pretty big lift, so this PR aims to just get something running in CI, even though it's only a fraction of the tests that are passing (and I've fudged a bunch of the matchers, so a bunch of the passing tests are fake). After this PR, there will be plenty more:
Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
GUS work item