Make unit tests run in a browser #1622
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Our tests run with the test framework `vitest`, which is for now using the `webdriverio` dependency to run it on real browser environments. However, it seems that `vitest` is using a deprecated API for each tests resulting in `webdriverio` outputing a warning log after each of them. Considering that we have thousands of tests, it makes the outputs of integration tests unreadable, output which is particularly important when running test scripts. This could be considered as a `vitest` issue, and they have been made aware of it through their #6804 issue (I'm not linking it in any way here as GitHub as the annoying tendency to spam messages to the original issue when linked that way, but it's the one named "`switchToFrame` deprecated in WebdriverIO v9"). But in the meantime it has been several month and it's hard to write and run integration tests with that mess. So here I'm proposing to just uglily patch console functions in `vitest`'s "globalSetup" script.
Our integration tests import the built RxPlayer as they test the final behavior and API of the RxPlayer, post-build scripts and all. I consequently hesitated to force a building step before calling the integration tests scripts (or even to make it a step inside those integration tests - e.g. through `vitest`'s "globalSetup"" concept) but finally refrain for that both in the same of simplicity (`npm run test:integration` just performs integration tests) and "performance" (you might just want to re-call `npm run test:integration` after fixing/adding new tests in which case there's no need to re-build the RxPlayer). Instead of alternatives like adding a poorly-discoverable option/env, a prompt or some documentation somewhere on this I here propose to just print a warning notice each time integration tests are run notifying that the wanted RxPlayer build must have been produced first. I did this at the `package.json` level, though I could have done it at `vitest`'s globalSetup level. I chose the `package.json` approach for now because a developer this way has a chance of seeing that warning before actually running the test:integration` script just by looking at available scripts.
I'm PoCing a common config for most kinds of tests in the RxPlayer
(integration, memory and unit tests, so we just exclude "performance"
and "conformance" tests).
All three of them are relying on `vitest` but integration+memory tests
run in a browser and unit tests run in a jsdom-ed Node.js environment.
With the goal of performing every tests in a web browser/user-agent to
better reflect actual RxPlayer usage (in production, we need to run on
something that has HTML5 video/audio and preferably EME+MSE, so Node.js
isn't a realist target today) and of having the simplicity of just being
able to call `npx vitest` to run all those tests with a common
environment, I'm currently updating `vitest` configs and unit tests.
This commit is an offshoot of that work that is compatible to what we
already do today:
- `vitest`'s config more explicitly include memory+integration tests.
- I added a default browser (sadly, I chose here Chrome, for
popularity and universality reasons) for when no browser is asked
for (this is only useful when calling `npx vitest` directly, which
we never do but which a developer could want to do - in which case
running all tests in chrome would be better than just failing
randomly).
peaBerberian
force-pushed
the
misc/vitest-browser2
branch
from
3e0d468 to
25327f8
Compare
peaBerberian
force-pushed
the
misc/vitest-browser2
branch
from
25327f8 to
f7d5638
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #1620, #1617, #1618 and #1619!
This mega-PR rewrites unit tests so they can be run in a web browser (for now Chrome + Firefox), through
webdriverio- just like our integration and memory tests.This allows us to run unit tests in the actual environment where the RxPlayer will run and to have a common script + environment for unit, integration and memory tests, thus simplifying the project.
The main difficulty was linked to our over-reliance on
vitest'sdoMock+importActualfunctions, the second which has a different behavior when run in a web browser: multipleimportActualcalls in the same test file won't re-evaluate that imported file. This means thatdoMockperformed in-betweenimportActualcalls for the same file won't have an impact on that file.This might be fixed in the future, but I wanted to see how most people succeeded to run
vitestin a browser. It seems that relying on a top-levelmockfunction is much more idiomatic, while relying on thehoistedfunction to also declare hoisted mocks whose implementation can change in-between tests.Though, to ensure portability between test frameworks if we ever change it (JavaScript is like that sometimes), I try to limit reliance on that
vitest-specificvi.mock/vi.hoistedconcept, by prefering just mocking through only the more portablespyOnwhen possible.