Proposal: move unit tests to tests/unit#1841
Open
peaBerberian wants to merge 1 commit into
Open
Conversation
peaBerberian
force-pushed
the
unit-tests-moved
branch
from
772e7bb to
bab306a
Compare
peaBerberian
commented
Apr 30, 2026
| // I did not find for example how to exclude some files (our unit tests) | ||
| // easily by running typescript directly from NodeJS. | ||
| // So we just spawn a separate process running tsc: | ||
| // We just spawn a separate process running tsc: |
Collaborator
Author
There was a problem hiding this comment.
Note: text not needed anymore here because we do not exclude unit tests at build now: they aren't included by our root tsconfig.json anymore
peaBerberian
force-pushed
the
unit-tests-moved
branch
2 times, most recently
from
77affe2 to
cda052c
Compare
peaBerberian
force-pushed
the
unit-tests-moved
branch
from
cda052c to
c19b41e
Compare
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Current unit test situation
Until now, unit tests have been alongside source files as
__tests__/*.test.tsfiles relative to the files they test.Lately, I spent some efforts to make them easier to write: mainly I added re-usable mocks to simplify the reliance on many modules, such as a PlaybackObserver (which emits playback metrics), doing fake but valid Manifest generation etc.
It was difficult to know how to architecture this alternative co-located code, leading to a few deep arcane imports here and there. I had a few issues already, like the previous time I did a dev release, where a source file mistakenly imported some test-only mock (it was a barrel file which mistakenly pleased and source files, and tests).
Proposal
Instead, I decided to take another road, which is the one taken by all other test suites (integration, memory, performance, conformance) in the RxPlayer: putting them in its own
/tests/directory, here/tests/unit/.Key points:
The root
tsconfig.jsonhad complex inclusion/exclusion rules mostly due to unit tests. By moving them I simplified it and removed the awkwardtsconfig.unit-tests.jsonfile in root by the better-placedtests/unit/tsconfig.jsonwhich is now consequently automatically picked up by LSP in most editors (contrarily to before)Unit tests are now in
tests/unit/src/where they mirror the path of the source file it tests. This is checked by CI by a new script (which saw that a few tests already were not named accordingly or did not follow some moved file)mocks are in
tests/unit/mocksUnit tests imports are relative, which may lead to few awkward paths, like
../../../../src/.I could have made a typescript/vitest alias here, but I don't think this very minor disagreement (even more with LSP) justify the added complexity for naive code analysis tooling.