Added tests for live query #12
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @Wojtach, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request adds three new tests for live queries in the cblite-tests/e2e/query-test.ts file. These tests cover scenarios such as adding a change listener to a query, verifying that a live query doesn't update when there are no changes, and handling empty result sets. One test related to updating query parameters is currently commented out, pending a fix for an existing issue.
Highlights
- New Tests: Three new tests have been added to
cblite-tests/e2e/query-test.tsto improve the coverage of live query functionality. - testQueryAddChangeListener: This test verifies that a change listener is properly added to a query and that it receives updates when the underlying data changes.
- testLiveQueryNoUpdate: This test ensures that a live query doesn't trigger updates when there are no changes to the data.
- testLiveQueryReturnsEmptyResultSet: This test checks the behavior of a live query when it initially returns an empty result set and how it reacts to subsequent data changes (in this case, a document purge).
Changelog
- cblite-tests/e2e/query-test.ts
- Added
testQueryAddChangeListenerto verify change listener functionality. - Added
testLiveQueryNoUpdateto ensure no updates when data is unchanged. - Added
testLiveQueryReturnsEmptyResultSetto test empty result set scenarios. - Commented out
testLiveQueryUpdateQueryParamdue to an existing issue.
- Added
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A query's alive,
Listening for each small change,
Data's heartbeat felt.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request adds tests for live queries, covering scenarios such as adding a change listener, handling no updates, and managing empty result sets. The tests seem well-structured and cover important aspects of live query functionality. However, there are a few areas where improvements can be made to enhance clarity and robustness.
Summary of Findings
- Inconsistent use of
sleep: Thesleepfunction is used with different durations in different tests. Standardizing the sleep duration or using a more dynamic approach based on the expected operation time could improve test reliability. - Missing test case for query parameter updates: The test case
testLiveQueryUpdateQueryParamis commented out due to a known issue. It would be beneficial to re-enable this test once the underlying issue is resolved to ensure complete coverage of live query functionality. - Potential race conditions in listener calls: The tests rely on
sleepto allow the listener to be called. This can be unreliable and lead to race conditions. Consider using a more robust synchronization mechanism, such as a promise or a semaphore, to ensure that the listener is called before proceeding with the assertions.
Merge Readiness
The pull request introduces valuable tests for live queries. However, the potential race conditions in listener calls and the commented-out test case for query parameter updates should be addressed before merging. I am unable to approve this pull request, and recommend that others review and approve this code before merging.
| changeCount = 0; | ||
| await this.defaultCollection.save(doc); | ||
|
|
||
| await this.sleep(1000); |
There was a problem hiding this comment.
Consider using a smaller sleep duration, or even better, use a promise that resolves when the listener has been called the expected number of times. This will make the test more reliable and faster.
| await this.sleep(1000); | |
| await this.sleep(100); |
| // Purge the document with id doc-1 | ||
| await this.defaultCollection.purgeById('doc-1'); | ||
|
|
||
| await this.sleep(1000); |
There was a problem hiding this comment.
Consider using a smaller sleep duration, or even better, use a promise that resolves when the listener has been called the expected number of times. This will make the test more reliable and faster.
| await this.sleep(1000); | |
| await this.sleep(1000); |
| // TODO when query parameters will be fixed | ||
| // https://github.com/Couchbase-Ecosystem/cbl-reactnative/issues/22 |
There was a problem hiding this comment.
It's good to see that this test is marked as TODO with a link to the relevant issue. Once the issue is resolved, remember to uncomment and re-enable this test to ensure full coverage.
| // TODO when query parameters will be fixed | |
| // https://github.com/Couchbase-Ecosystem/cbl-reactnative/issues/22 | |
| // TODO when query parameters will be fixed | |
| // https://github.com/Couchbase-Ecosystem/cbl-reactnative/issues/22 |
| changeCount = 0; | ||
| await this.defaultCollection.save(doc); |
There was a problem hiding this comment.
Why is changeCount being reset to 0 here? It seems like this could lead to incorrect assertions later on. Consider removing this line if it's not necessary.
| changeCount = 0; | |
| await this.defaultCollection.save(doc); | |
| changeCount = 0; | |
| await this.defaultCollection.save(doc); |
No description provided.