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

refactor(test): cleanup observable tests #949

Merged
merged 2 commits into from
Jan 3, 2025
Merged

refactor(test): cleanup observable tests #949

merged 2 commits into from
Jan 3, 2025

Conversation

bjoerge
Copy link
Member

@bjoerge bjoerge commented Jan 3, 2025

This simplifies the tests for client.listen and client.live, making them less verbose and (hopefully) a bit easier to follow.

Also added a test to assert that the client sends last-event-id header in case of a disconnect.

Also fixed a couple of warnings that was issued during test runs due to lack of awaiting (or returning) promises at the end of a few tests:

Promise returned by `expect(actual).rejects.toBeDefined()` was not awaited. Vitest currently auto-awaits hanging assertions at the end of the test, but this will cause the test to fail in Vitest 3. Please remember to await the assertion.
    at client/test/client.test.ts:429:46

++ a couple of other minor improvements

@bjoerge bjoerge requested a review from stipsan January 3, 2025 12:08
Copy link
Member

@stipsan stipsan left a comment

Choose a reason for hiding this comment

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

Much easier to follow indeed 💖

@@ -2600,7 +2576,7 @@ describe('client', async () => {
'data: {"listenerName":"LGFXwOqrf1GHawAjZRnhd6"}',
'',
'event: mutation',
`data: ${JSON.stringify({document: doc})}`,
`data: ${JSON.stringify({result: doc})}`,
Copy link
Member Author

Choose a reason for hiding this comment

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

this aligns with mutation events from /listen endpoint returning result, not document

@@ -140,6 +116,37 @@ describe.skipIf(typeof EdgeRuntime === 'string' || typeof document !== 'undefine
}
})

test('sends last-event-id header when reconnecting', async () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

buried lede, perhaps, but this change is in a separate commit, so we might want merge with rebase to keep this as a separate item in the commit history.

@bjoerge bjoerge added this pull request to the merge queue Jan 3, 2025
Merged via the queue into main with commit ebe12a2 Jan 3, 2025
13 checks passed
@bjoerge bjoerge deleted the cleanup-tests branch January 3, 2025 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants