Clean up our tests in preparation for the testing sprint

[robintown]

See the commit messages for the details!

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 3.25806% with 2999 lines in your changes missing coverage. Please review.

Project coverage is 17.13%. Comparing base (8cced48) to head (31a1c5e).
Report is 554 commits behind head on livekit.

Files	Patch %	Lines
src/grid/Grid.tsx	0.00%	481 Missing ⚠️
src/room/InCallView.tsx	0.00%	388 Missing ⚠️
src/tile/SpotlightTile.tsx	0.00%	323 Missing ⚠️
src/tile/GridTile.tsx	0.00%	301 Missing ⚠️
src/state/CallViewModel.ts	0.00%	297 Missing ⚠️
src/state/MediaViewModel.ts	0.00%	160 Missing ⚠️
src/grid/CallLayout.ts	0.00%	152 Missing ⚠️
src/grid/GridLayout.tsx	0.00%	135 Missing ⚠️
src/tile/MediaView.tsx	0.00%	127 Missing ⚠️
src/grid/SpotlightPortraitLayout.tsx	0.00%	119 Missing ⚠️
... and 25 more

Additional details and impacted files

@@             Coverage Diff             @@
##           livekit    #2466      +/-   ##
===========================================
- Coverage    25.02%   17.13%   -7.89%     
===========================================
  Files           47      143      +96     
  Lines         2382    19297   +16915     
  Branches       434      249     -185     
===========================================
+ Hits           596     3307    +2711     
- Misses        1735    15990   +14255     
+ Partials        51        0      -51     

Flag	Coverage Δ
unittests	17.13% <3.25%> (-7.89%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[AndrewFerr]

Regarding f79ca02, could we instead just change the linters' search paths to find test files? Personally I prefer having test files in their own directory, as it makes it easier to separate them from source files & many of our projects put tests in a separate directory (aside from the Rust projects where tests are within source files).

[robintown]

Hmm, my experience is that separation from the source files, at least for unit tests, just adds friction. When re-organizing files or directories, it becomes really easy to find the test files and make sure you're moving them at the same time as the code. I believe this is one of the reasons why you see Rust and Vitest even moving tests into the source file nowadays.

@toger5 Do you have any preference here?

[toger5]

I like it when the tests are on the same level where they test. I like if a function that does not require any context mocking but just validates a type for instance does not leave the scope of the file. So having the test in the file is nice here.
But as soon as there is mocking setup that might be useful for multiple files a dedicated test file makes sense. Then all mocking/helper code is at one spot and does not pollute the actual source.

This comment is not too helpful to find an immediate solution I think. It depends on what kind of tests we are mostly writing.

[robintown]

Concluded in stand-up to keep the test files inside src, as I've done here. This is ready for review then

[robintown]

Turning on a coverage gate for Codecov was pretty easy once I managed to find the right page of the docs: https://docs.codecov.com/docs/commit-status#patch-status

I've included that coverage gate configuration in this PR.

[toger5]

Welp, I've rebased the PR and now the tests are throwing spurious errors in CI that I can't reproduce locally. You can see a bunch of commits above from me trying to fix it. Unfortunately, I'll need to leave that for someone else to figure out, as my week is about over.

I am looking into this. Initially I was able to reproduce the issues locally. After some time I also had them running without changing sth significantly. One finding is that having multiple async tests results in running the tests failing and having only one async seems to work. (but I am not sure if this is just random with a bias after all)

After some iterations I concluded the following:

• The CI is probably not doing multithreaded tests and running yarn vitest --no-file-parallelism results in the same errors locally and on the remote.
• All tests using the userEvent module cause errors in the test suit that make it fail (at least how we do them now)
• Since my local test runner was suffering from multi thread flakyness I played around with component un-mounting. The result is the obvious, that a const {unmount} = render(...) behaves equivalent to a afterEach(cleanup)

[toger5]

This is missing two tests now but has all the minor things fixed to get through ci.