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.

Sign up for GitHub

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

Jump to bottom

Improve the integration test coverage for online vs offline comparisons. #6841

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve the integration test coverage for online vs offline comparisons. #6841

wants to merge 1 commit into from

Conversation

ehsannas
Copy link
Contributor

@ehsannas ehsannas commented Apr 4, 2025 •
edited
Loading

This PR improves the way checkOnlineAndOfflineResultsMatch test util function was written.

It does so by first pre-populating the cache with the entire collection, then performing the query from cache (this results in a full collection scan), then performing the query from the server, and then performing the query from cache again (this results in using performQueryUsingRemoteKeys. It then ensures that all of these 3 results are the same and equal to the expected results.

#no-changelog

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 4, 2025 •
edited
Loading

📝 PRs merging into main branch

Our 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.

@google-oss-bot
Copy link
Contributor

1 Warning
⚠️ Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger

@ehsannas ehsannas requested a review from milaGGL April 4, 2025 23:44
@google-oss-bot
Copy link
Contributor

Coverage Report 1

Affected Products

  • firebase-firestore

    Overall coverage changed from 45.77% (8af38c9) to 45.76% (e53d241) by -0.01%.

    FilenameBase (8af38c9)Merge (e53d241)Diff
    DeleteMutation.java90.48%95.24%+4.76%
    LruGarbageCollector.java97.27%93.64%-3.64%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/fK01aS3Xjt.html

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 4, 2025

Test Results

  186 files  +  120    186 suites  +120   4m 26s ⏱️ + 3m 8s
1 235 tests +  683  1 219 ✅ +  668  16 💤 +15  0 ❌ ±0 
2 494 runs  +1 390  2 462 ✅ +1 360  32 💤 +30  0 ❌ ±0 

Results for commit 8567041. ± Comparison against base commit 8af38c9.

This pull request removes 552 and adds 1235 tests. Note that renamed tests count towards both. 
com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ descriptor should have expected values
com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ deserialize() should throw UnsupportedOperationException
com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ serialize() should throw UnsupportedOperationException
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Boolean) creates an object with the expected value
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Double) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Double) creates an object with the expected value (normal cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(List) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(List) creates an object with the expected value (normal cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Map) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Map) creates an object with the expected value (normal cases)
…

com.google.firebase.firestore.AggregateQuerySnapshotTest ‑ createWithCountShouldReturnInstanceWithTheGivenQueryAndCount
com.google.firebase.firestore.AggregateQueryTest ‑ testSourceMustNotBeNull
com.google.firebase.firestore.BlobTest ‑ testComparison
com.google.firebase.firestore.BlobTest ‑ testEquals
com.google.firebase.firestore.BlobTest ‑ testMutableBytes
com.google.firebase.firestore.CollectionReferenceTest ‑ testEquals
com.google.firebase.firestore.DocumentChangeTest ‑ randomTests
com.google.firebase.firestore.DocumentChangeTest ‑ testAdditions
com.google.firebase.firestore.DocumentChangeTest ‑ testChangesWithSortOrderChange
com.google.firebase.firestore.DocumentChangeTest ‑ testDeletions
…
This pull request removes 1 skipped test and adds 16 skipped tests. Note that renamed tests count towards both. 
com.google.firebase.dataconnect.ProtoStructDecoderUnitTest ‑ decodeFromStruct() can encode and decode a list of non-nullable Unit

com.google.firebase.firestore.FirestoreRegistrarTest ‑ storageRegistrar_getComponents_publishesLibVersionComponent
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testDoesNotReplaceResumeTokenWithEmptyByteString
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testHandlesSetMutationThenAckThenTransformThenAckThenTransform
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testIgnoresTargetMappingAfterExistenceFilterMismatch
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testPersistsResumeTokens
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testQueriesFilterDocumentsThatNoLongerMatch
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testQueriesIncludeDocumentsFromOtherQueries
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testQueriesIncludeLocallyModifiedDocuments
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testUsesTargetMappingToExecuteQueries
com.google.firebase.firestore.local.SQLiteLocalStoreTest ‑ testCollectsGarbageAfterAcknowledgedMutation
…

@google-oss-bot
Copy link
Contributor

Size Report 1

Affected Products

No changes between base commit (8af38c9) and merge commit (e53d241).

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/SMVFEIq4sX.html

@google-oss-bot
Copy link
Contributor

Startup Time Report 1

Note: Layout is sometimes suboptimal due to limited formatting support on GitHub. Please check this report on GCS.

Notes

Startup Times

  • fire-fst

    DeviceStatisticsDistributions
    oriole-32
    Percentile8af38c9e53d241DiffSignificant (?)
    p10266 ±10 μs260 ±14 μs-6.48 μs (-2.4%)NO
    p25274 ±12 μs268 ±16 μs-6.14 μs (-2.2%)NO
    p50289 ±23 μs284 ±21 μs-4.62 μs (-1.6%)NO
    p75342 ±76 μs328 ±61 μs-13.9 μs (-4.1%)NO
    p90390 ±103 μs423 ±136 μs+33.6 μs (+8.6%)NO

    20 test runs in comparison
    CommitTest Runs
    8af38c9
    • 2025-04-04_20:07:26.635355_LoHJ
    • 2025-04-04_20:07:26.635392_uJrM
    • 2025-04-04_20:07:26.635402_JjSr
    • 2025-04-04_20:07:26.635409_AwrL
    • 2025-04-04_20:07:26.635417_JtHz
    • 2025-04-04_20:07:26.635424_wMAx
    • 2025-04-04_20:07:26.635430_cFVv
    • 2025-04-04_20:07:26.635436_CZKg
    • 2025-04-04_20:07:26.635443_AEVE
    • 2025-04-04_20:07:26.635449_Irio
    e53d241
    • 2025-04-04_23:56:07.690936_VDLS
    • 2025-04-04_23:56:07.690975_PndE
    • 2025-04-04_23:56:07.691004_jjvW
    • 2025-04-04_23:56:07.691013_AejR
    • 2025-04-04_23:56:07.691020_KWQC
    • 2025-04-04_23:56:07.691027_BCAG
    • 2025-04-04_23:56:07.691034_rPsT
    • 2025-04-04_23:56:07.691041_Vrze
    • 2025-04-04_23:56:07.691047_Brar
    • 2025-04-04_23:56:07.691053_nhfe
    redfin-30
    Percentile8af38c9e53d241DiffSignificant (?)
    p10480 ±20 μs520 ±66 μs+39.4 μs (+8.2%)NO
    p25492 ±20 μs539 ±73 μs+47.4 μs (+9.6%)NO
    p50510 ±24 μs565 ±89 μs+54.7 μs (+10.7%)NO
    p75531 ±28 μs600 ±117 μs+69.0 μs (+13.0%)NO
    p90558 ±36 μs733 ±417 μs+175 μs (+31.4%)NO

    20 test runs in comparison
    CommitTest Runs
    8af38c9
    • 2025-04-04_20:07:26.635355_LoHJ
    • 2025-04-04_20:07:26.635392_uJrM
    • 2025-04-04_20:07:26.635402_JjSr
    • 2025-04-04_20:07:26.635409_AwrL
    • 2025-04-04_20:07:26.635417_JtHz
    • 2025-04-04_20:07:26.635424_wMAx
    • 2025-04-04_20:07:26.635430_cFVv
    • 2025-04-04_20:07:26.635436_CZKg
    • 2025-04-04_20:07:26.635443_AEVE
    • 2025-04-04_20:07:26.635449_Irio
    e53d241
    • 2025-04-04_23:56:07.690936_VDLS
    • 2025-04-04_23:56:07.690975_PndE
    • 2025-04-04_23:56:07.691004_jjvW
    • 2025-04-04_23:56:07.691013_AejR
    • 2025-04-04_23:56:07.691020_KWQC
    • 2025-04-04_23:56:07.691027_BCAG
    • 2025-04-04_23:56:07.691034_rPsT
    • 2025-04-04_23:56:07.691041_Vrze
    • 2025-04-04_23:56:07.691047_Brar
    • 2025-04-04_23:56:07.691053_nhfe

  • timeToInitialDisplay

    DeviceStatisticsDistributions
    oriole-32
    Percentile8af38c9e53d241DiffSignificant (?)
    p10196 ±5 ms198 ±3 ms+1.63 ms (+0.8%)NO
    p25203 ±5 ms204 ±3 ms+1.00 ms (+0.5%)NO
    p50209 ±5 ms211 ±4 ms+2.25 ms (+1.1%)NO
    p75217 ±5 ms219 ±3 ms+2.18 ms (+1.0%)NO
    p90224 ±6 ms227 ±5 ms+3.60 ms (+1.6%)NO

    20 test runs in comparison
    CommitTest Runs
    8af38c9
    • 2025-04-04_20:07:26.635355_LoHJ
    • 2025-04-04_20:07:26.635392_uJrM
    • 2025-04-04_20:07:26.635402_JjSr
    • 2025-04-04_20:07:26.635409_AwrL
    • 2025-04-04_20:07:26.635417_JtHz
    • 2025-04-04_20:07:26.635424_wMAx
    • 2025-04-04_20:07:26.635430_cFVv
    • 2025-04-04_20:07:26.635436_CZKg
    • 2025-04-04_20:07:26.635443_AEVE
    • 2025-04-04_20:07:26.635449_Irio
    e53d241
    • 2025-04-04_23:56:07.690936_VDLS
    • 2025-04-04_23:56:07.690975_PndE
    • 2025-04-04_23:56:07.691004_jjvW
    • 2025-04-04_23:56:07.691013_AejR
    • 2025-04-04_23:56:07.691020_KWQC
    • 2025-04-04_23:56:07.691027_BCAG
    • 2025-04-04_23:56:07.691034_rPsT
    • 2025-04-04_23:56:07.691041_Vrze
    • 2025-04-04_23:56:07.691047_Brar
    • 2025-04-04_23:56:07.691053_nhfe
    redfin-30
    Percentile8af38c9e53d241DiffSignificant (?)
    p10225 ±4 ms261 ±30 ms+36.3 ms (+16.1%)NO
    p25231 ±4 ms269 ±31 ms+37.7 ms (+16.3%)NO
    p50239 ±5 ms278 ±32 ms+39.1 ms (+16.3%)NO
    p75246 ±6 ms289 ±32 ms+42.7 ms (+17.3%)NO
    p90257 ±12 ms308 ±30 ms+51.0 ms (+19.8%)NO

    20 test runs in comparison
    CommitTest Runs
    8af38c9
    • 2025-04-04_20:07:26.635355_LoHJ
    • 2025-04-04_20:07:26.635392_uJrM
    • 2025-04-04_20:07:26.635402_JjSr
    • 2025-04-04_20:07:26.635409_AwrL
    • 2025-04-04_20:07:26.635417_JtHz
    • 2025-04-04_20:07:26.635424_wMAx
    • 2025-04-04_20:07:26.635430_cFVv
    • 2025-04-04_20:07:26.635436_CZKg
    • 2025-04-04_20:07:26.635443_AEVE
    • 2025-04-04_20:07:26.635449_Irio
    e53d241
    • 2025-04-04_23:56:07.690936_VDLS
    • 2025-04-04_23:56:07.690975_PndE
    • 2025-04-04_23:56:07.691004_jjvW
    • 2025-04-04_23:56:07.691013_AejR
    • 2025-04-04_23:56:07.691020_KWQC
    • 2025-04-04_23:56:07.691027_BCAG
    • 2025-04-04_23:56:07.691034_rPsT
    • 2025-04-04_23:56:07.691041_Vrze
    • 2025-04-04_23:56:07.691047_Brar
    • 2025-04-04_23:56:07.691053_nhfe

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/XamWpUlDwo/index.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@milaGGL milaGGL Awaiting requested review from milaGGL

At least 1 approving review is required to merge this pull request.

Assignees

@milaGGL milaGGL

Labels
no-changelog size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ehsannas @google-oss-bot @milaGGL