-
Notifications
You must be signed in to change notification settings - Fork 580
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
Increase test coverage for persistent cache indexing #5213
Conversation
📝 PRs merging into main branchOur main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released. |
003fb22
to
b85f853
Compare
Coverage Report 1Affected Products
Test Logs |
Unit Test Results 164 files - 656 164 suites - 656 2m 43s ⏱️ - 36m 5s Results for commit b6c3465. ± Comparison against base commit f2a0fa3. ♻️ This comment has been updated with latest results. |
Size Report 1Affected Products
Test Logs |
Startup Time Report 1Note: Layout is sometimes suboptimal due to limited formatting support on GitHub. Please check this report on GCS. Notes
Startup Times
|
99b25a4
to
09498d1
Compare
a921927
to
d326e29
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one tiny nit about line length.
Note that in the web port of client-side index auto creation (firebase/firebase-js-sdk#7542) I added "testing hooks" that provide a way for integration tests to verify that the indexes are (or are not) created, as expected. This is something we should consider porting to android and ios IMO. The "testing hooks" infrastructure exists in android and ios too (it was created for the bloom filter work) so we'd just need to add to it.
firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java
Outdated
Show resolved
Hide resolved
As discussed in the meeting, testing hook will be added in a separated PR. Tracking number: b/296100693 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor nits, but mostly LGTM.
firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java
Outdated
Show resolved
Hide resolved
firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java
Show resolved
Hide resolved
...base-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteIndexManagerTest.java
Outdated
Show resolved
Hide resolved
firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java
Show resolved
Hide resolved
firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java
Show resolved
Hide resolved
firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java
Show resolved
Hide resolved
87685ff
to
b6c3465
Compare
No description provided.