Skip to content

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

Merged
ehsannas merged 1 commit intomainfrom
ehsann/better-test-utils
Apr 10, 2025
Merged

Improve the integration test coverage for online vs offline comparisons.#6841
ehsannas merged 1 commit intomainfrom
ehsann/better-test-utils

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
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
Collaborator

google-oss-bot commented Apr 4, 2025

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
Collaborator

google-oss-bot commented Apr 4, 2025

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
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
Collaborator

google-oss-bot commented Apr 4, 2025

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
Collaborator

google-oss-bot commented Apr 5, 2025

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

milaGGL
milaGGL approved these changes Apr 10, 2025
View reviewed changes
Copy link
Contributor

@milaGGL milaGGL left a comment

Choose a reason for hiding this comment

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

LGTM

@ehsannas ehsannas merged commit 70c8e89 into main Apr 10, 2025
33 of 34 checks passed
@ehsannas ehsannas deleted the ehsann/better-test-utils branch April 10, 2025 19:25
ehsannas added a commit to firebase/firebase-ios-sdk that referenced this pull request Apr 10, 2025
@ehsannas
Improve the integration test coverage for online vs offline comparisons.
7e0049f 
Ports firebase/firebase-android-sdk#6841 to iOS.
@ehsannas ehsannas mentioned this pull request Apr 10, 2025
Merged
ehsannas added a commit to firebase/firebase-ios-sdk that referenced this pull request Apr 10, 2025
@ehsannas
Improve the integration test coverage for online vs offline comparisons.
06091bc 
Ports firebase/firebase-android-sdk#6841 to iOS.
@firebase firebase locked and limited conversation to collaborators May 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@milaGGL milaGGL milaGGL approved these changes

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