-
Notifications
You must be signed in to change notification settings - Fork 294
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
Run every test with and without scoped-element-registry polyfill #2353
base: master
Are you sure you want to change the base?
Run every test with and without scoped-element-registry polyfill #2353
Conversation
|
Hi @aghArdeshir , |
Intersting @okadurin . I see now what we are trying to test. Yes I may be able to look into this. I'll let you know here if I could come up with a solution or if I eventually gave up |
I believe I found a better approach @okadurin : https://modern-web.dev/docs/test-runner/cli-and-configuration/#customize-html-environment-per-test-group I can duplicate our test groups ( |
cff40ad
to
3a6f625
Compare
cb4f632
to
0a3b955
Compare
remove unnecessary polyfill loading scripts that need polyfill import themselves + unnecessary mocking removal separate tests that require polyfill from tests that do not remove unneeded test web test runner create groups for tests to run with and without polyfills refactor: rename
0a3b955
to
2e883d6
Compare
@okadurin I've updated my MR and have also updated my original message (#2353 (comment)) to list what changes I made. Take another look? |
This PR fixes #2351
testGroups
to achieve thissome-test.polyfill.test.js
andsome-test.no-polyfill.test.js
for tests that require to be run only with/without the polyfill. (Example is the SlotMixin tests).polyfill
, because that needed polyfill to pass. Also refactored it to use a simplespy
instead of a mocked object.does not scope elements when polyfill not loaded
but when you actually remove the polyfill from test config, it fails! (I gave more explanation here: Remove SlotMixin Test with wrong assumption about polyfill being loaded #2358)UPDATE: Below is the original message of MR, which is not accurate anymore. This PR is re-purposed.
- We should not load a polyfill globally in tests environment, because it affects how tests behave and may give us false positive and false confidence.- Every test that needs polyfill, loads the polyfill only for itself.- No unnecessary mocking is required. Spy would do the job in this scneario.- Adds a util for adding a script in tests on demand.- Removes one unnecessary ts-ignore