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

Switch unit tests to use vitest #1445

Merged
merged 12 commits into from
Jun 17, 2024
Merged

Switch unit tests to use vitest #1445

merged 12 commits into from
Jun 17, 2024

Conversation

peaBerberian
Copy link
Collaborator

@peaBerberian peaBerberian commented Jun 4, 2024

To extrapolate on #1444, this PR shows how it would look like to switch from relying on Jest to rely on vitest for our unit tests.

The end goal being to simplify our codebase by relying on a single testing framework.

It can be tested right now by calling npm run test:unit.

Sadly for now, we are still forced to rely on a JSDom-ed Node.js environment for unit tests and a browser environment for integration tests - meaning we have very different configs for both.

This is because we want to mock imported files in unit tests - something that is not possible for now in browser environment through vitest (though vitest/5765 [any real link seems to spam their GitHub page, even with a random anchor added to it] seems to have been merged very recently so maybe we could rely on the browser for both soon), yet we want to replicate as much as a real browser as possible in our integration tests (because we're also testing that media playback on the tested browsers goes as expected).

@peaBerberian peaBerberian force-pushed the refactor/vitest-unit-test branch 5 times, most recently from 0df9add to 83fdeb6 Compare June 4, 2024 15:17
.eslintrc.js Outdated Show resolved Hide resolved
Comment on lines +1 to +2
import { describe, beforeEach, afterEach, it, expect, vi } from "vitest";
import arrayFindIndex from "../array_find_index";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if the copyright header needs to be on every file, but it seems it has been removed by mistake ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No I originally added them because it was there in other files, kind of out of respect for my elders basically :p

Yet we already have a global LICENSE file and those notices clutter the files too much, so now that we technically refactor those files, I profit from it by removing those - silently enough as not to wake up any potential over-zealous lawyer 🤫

@peaBerberian peaBerberian force-pushed the refactor/vitest-unit-test branch 16 times, most recently from 3fdcc38 to e5734e5 Compare June 4, 2024 17:53
@peaBerberian peaBerberian added tests Relative to the RxPlayer's tests Priority: 3 (Low) This issue or PR has a low priority. labels Jun 10, 2024
@peaBerberian peaBerberian force-pushed the refactor/vitest-unit-test branch 6 times, most recently from d1422a2 to 63d295e Compare June 10, 2024 17:59
@peaBerberian peaBerberian changed the title [WIP] [Proposal] Switch unit tests to use vitest Switch unit tests to use vitest Jun 11, 2024
@peaBerberian peaBerberian added this to the 4.1.0 milestone Jun 11, 2024
@peaBerberian peaBerberian force-pushed the refactor/replace-karma-by-vitest branch from abc6a78 to 4d40b98 Compare June 11, 2024 11:50
@Florent-Bouisset Florent-Bouisset force-pushed the refactor/replace-karma-by-vitest branch 2 times, most recently from 64250eb to c554adb Compare June 12, 2024 11:31
@peaBerberian peaBerberian force-pushed the refactor/vitest-unit-test branch 3 times, most recently from e368c3d to b4fe9e7 Compare June 13, 2024 13:20
@peaBerberian peaBerberian mentioned this pull request Jun 13, 2024
@peaBerberian peaBerberian force-pushed the refactor/replace-karma-by-vitest branch from 2938954 to d40a343 Compare June 14, 2024 09:54
Florent-Bouisset and others added 12 commits June 14, 2024 12:55
To extrapolate on #1444, this PR shows how it would look like to switch
from relying on Jest to rely on vitest for our unit tests.

The end goal being to simplify our codebase by relying on a single
testing framework.

---

I only done so for testing files in the `src/compat` directory in this
demo. It can be tested right now by calling `npm run test:unit:vitest`.

---

Sadly for now, we are still forced to rely on a JSDom-ed Node.js
environment for unit tests and a browser environment for integration
tests - meaning we have very different configs for both.

This is because we want to mock imported files in unit tests - something
that is not possible for now in browser environment through vitest
(though vitest/5765 seems to have
been merged very recently so maybe we could rely on the browser for both
soon), yet we want to replicate as much as a real browser as possible in
our integration tests (because we're also testing that media playback on the
tested browsers goes as expected).
@peaBerberian peaBerberian changed the base branch from refactor/replace-karma-by-vitest to dev June 14, 2024 10:56
@peaBerberian peaBerberian merged commit 5794201 into dev Jun 17, 2024
6 checks passed
@peaBerberian peaBerberian deleted the refactor/vitest-unit-test branch July 26, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 3 (Low) This issue or PR has a low priority. tests Relative to the RxPlayer's tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants