Fix replication in tests #2614
Merged
Fix replication in tests #2614
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
raineorshine
approved these changes
Nov 23, 2024
src/components/__tests__/Bullet.ts
Outdated
| @@ -26,6 +26,8 @@ describe('render', () => { | |||
| }), | |||
| ]) | |||
|
|
|||
| await act(async () => vi.runOnlyPendingTimersAsync()) | |||
There was a problem hiding this comment.
FYI This can be shortened to:
await act(vi.runOnlyPendingTimersAsync)| @@ -45,10 +42,12 @@ it('disable isLoading after initialize', async () => { | |||
| }) | |||
|
|
|||
| // y-indexeddb breaks tests | |||
| it.skip('load thought', async () => { | |||
| it('load thought', async () => { | |||
There was a problem hiding this comment.
Very exciting! It's been a long time since these tests have been operational. It will also allow us to write new tests that cover more complex persistence behavior.
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.
Replication via
thoughtspace.tswas broken in the tests, so replication was explicitly prevented when running the tests. This appears to have allowed the majority of the tests to work since replication is not required for a lot of them but some tests that rely on the replication failed and were skipped.The primary problem appears to be that
vi.runPendingTimersdoes not appear to work for database replication tasks;await vi.runPendingTimersis required. I am not entirely sure why this is because I do not really understand the internals here.Once this was fixed it led to a cascade of other problems that had to be fixed. My guess is that once replication was enabled, asynchronous background functionality that was not being triggered suddenly got enabled and was causing asynchronous stuff to run after the tests had completed. This required going back and ensuring that all of that activity was properly awaited.
Given the volume of the problems I did not spend a significant amount of time analyzing each issue so some of the solutions may be non-optimal, but I think we should spend some time trying to decide if there are better ways to solve the problems that all the asynchronous execution seems to be causing before analyzing every test problem.
A miscellaneous note: I had to move some tests around; it looks like the tests that rely on
createTestStoremay not be getting completely cleaned up and thus leave remnants in the database that persist to later tests, causing them to break. I think there's probably a pretty easy fix here but it made sense to go with the quicker fix (move the breaking tests to the top of the file) instead of me figuring out the better fix.The operative changes are in
thoughtspace.tsandcreateTestStore.ts; most of the other changes are essentially making sure the fake timers are properly moved forward.