Skip to content

Firestore: QueryTest.java: improve the test that resumes a query with existence filter to actually validate the existence filter. #4813

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

Merged
merged 5 commits into from
Mar 24, 2023

Conversation

dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Mar 23, 2023

This PR improves the test resumingAQueryShouldUseExistenceFilterToDetectDeletes() in QueryTest.java, which was added by the previous PR #4799, to also validate that the existence filter sent from Watch was correct. This was achieved by adding some test-only "hooks" into the SDK's internals that enable the test to "listen" for existence filter mismatches. This is a port of firebase/firebase-js-sdk#7149 and firebase/firebase-ios-sdk#11209.

… existence filter to actually validate the existence filter.

This is a port of firebase/firebase-js-sdk#7149, and builds upon the test added in #4799.
@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2023

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

Add the 'main-merge-ack' label to your PR to confirm merging into the main branch is intended.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 23, 2023

Coverage Report 1

Affected Products

  • firebase-firestore

    Overall coverage changed from 44.18% (687e079) to 44.15% (926d683) by -0.03%.

    FilenameBase (687e079)Merge (926d683)Diff
    AutoValue_TestingHooks_ExistenceFilterMismatchInfo.java?20.00%?
    LocalStore.java100.00%99.39%-0.61%
    LruGarbageCollector.java97.20%93.46%-3.74%
    SetMutation.java94.44%97.22%+2.78%
    TestingHooks.java?40.91%?
    WatchChangeAggregator.java98.21%98.26%+0.04%

Test Logs

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

@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2023

Unit Test Results

   158 files  +   158     158 suites  +158   1m 59s ⏱️ + 1m 59s
1 117 tests +1 117  1 101 ✔️ +1 101  16 💤 +16  0 ±0 
2 234 runs  +2 234  2 202 ✔️ +2 202  32 💤 +32  0 ±0 

Results for commit f62d94f. ± Comparison against base commit 687e079.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 23, 2023

Size Report 1

Affected Products

  • firebase-firestore

    TypeBase (687e079)Merge (926d683)Diff
    aar1.32 MB1.32 MB+3.88 kB (+0.3%)
    apk (release)3.93 MB3.94 MB+1.53 kB (+0.0%)

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 23, 2023

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
    Percentile687e079926d683DiffSignificant (?)
    p10335 ±28 μs331 ±19 μs-3.90 μs (-1.2%)NO
    p25353 ±40 μs344 ±22 μs-9.13 μs (-2.6%)NO
    p50379 ±63 μs369 ±37 μs-10.5 μs (-2.8%)NO
    p75416 ±90 μs416 ±75 μs+76.4 ns (+0.0%)NO
    p90483 ±176 μs531 ±144 μs+47.9 μs (+9.9%)NO

    20 test runs in comparison
    CommitTest Runs
    687e079
    • 2023-03-23_16:39:37.443178_SHcH
    • 2023-03-23_16:39:37.445852_XCTk
    • 2023-03-23_16:39:37.445868_fJBT
    • 2023-03-23_16:39:37.445873_WBqI
    • 2023-03-23_16:39:37.445877_wdTg
    • 2023-03-23_16:39:37.445895_SLKB
    • 2023-03-23_16:39:37.445900_vfIH
    • 2023-03-23_16:39:37.445904_igIy
    • 2023-03-23_16:39:37.445908_KIUG
    • 2023-03-23_16:39:37.445912_EQdm
    926d683
    • 2023-03-24_02:38:58.707172_DhNU
    • 2023-03-24_02:38:58.708320_Flao
    • 2023-03-24_02:38:58.708331_qOuv
    • 2023-03-24_02:38:58.708337_Lhpp
    • 2023-03-24_02:38:58.708342_igkY
    • 2023-03-24_02:38:58.708347_dAPx
    • 2023-03-24_02:38:58.708353_QEAE
    • 2023-03-24_02:38:58.708358_ZPrE
    • 2023-03-24_02:38:58.708363_CrqE
    • 2023-03-24_02:38:58.708368_oOno
    redfin-30
    Percentile687e079926d683DiffSignificant (?)
    p10635 ±16 μs627 ±38 μs-7.22 μs (-1.1%)NO
    p25651 ±23 μs644 ±47 μs-7.19 μs (-1.1%)NO
    p50674 ±32 μs665 ±53 μs-9.04 μs (-1.3%)NO
    p75706 ±44 μs695 ±63 μs-11.5 μs (-1.6%)NO
    p90751 ±69 μs748 ±101 μs-2.28 μs (-0.3%)NO

    20 test runs in comparison
    CommitTest Runs
    687e079
    • 2023-03-23_16:39:37.443178_SHcH
    • 2023-03-23_16:39:37.445852_XCTk
    • 2023-03-23_16:39:37.445868_fJBT
    • 2023-03-23_16:39:37.445873_WBqI
    • 2023-03-23_16:39:37.445877_wdTg
    • 2023-03-23_16:39:37.445895_SLKB
    • 2023-03-23_16:39:37.445900_vfIH
    • 2023-03-23_16:39:37.445904_igIy
    • 2023-03-23_16:39:37.445908_KIUG
    • 2023-03-23_16:39:37.445912_EQdm
    926d683
    • 2023-03-24_02:38:58.707172_DhNU
    • 2023-03-24_02:38:58.708320_Flao
    • 2023-03-24_02:38:58.708331_qOuv
    • 2023-03-24_02:38:58.708337_Lhpp
    • 2023-03-24_02:38:58.708342_igkY
    • 2023-03-24_02:38:58.708347_dAPx
    • 2023-03-24_02:38:58.708353_QEAE
    • 2023-03-24_02:38:58.708358_ZPrE
    • 2023-03-24_02:38:58.708363_CrqE
    • 2023-03-24_02:38:58.708368_oOno
  • timeToInitialDisplay

    DeviceStatisticsDistributions
    oriole-32
    Percentile687e079926d683DiffSignificant (?)
    p10195 ±7 ms196 ±5 ms+1.74 ms (+0.9%)NO
    p25202 ±7 ms202 ±5 ms+271 μs (+0.1%)NO
    p50213 ±16 ms210 ±5 ms-2.99 ms (-1.4%)NO
    p75223 ±23 ms220 ±6 ms-2.58 ms (-1.2%)NO
    p90232 ±26 ms232 ±9.6 ms-53.1 μs (-0.0%)NO

    20 test runs in comparison
    CommitTest Runs
    687e079
    • 2023-03-23_16:39:37.443178_SHcH
    • 2023-03-23_16:39:37.445852_XCTk
    • 2023-03-23_16:39:37.445868_fJBT
    • 2023-03-23_16:39:37.445873_WBqI
    • 2023-03-23_16:39:37.445877_wdTg
    • 2023-03-23_16:39:37.445895_SLKB
    • 2023-03-23_16:39:37.445900_vfIH
    • 2023-03-23_16:39:37.445904_igIy
    • 2023-03-23_16:39:37.445908_KIUG
    • 2023-03-23_16:39:37.445912_EQdm
    926d683
    • 2023-03-24_02:38:58.707172_DhNU
    • 2023-03-24_02:38:58.708320_Flao
    • 2023-03-24_02:38:58.708331_qOuv
    • 2023-03-24_02:38:58.708337_Lhpp
    • 2023-03-24_02:38:58.708342_igkY
    • 2023-03-24_02:38:58.708347_dAPx
    • 2023-03-24_02:38:58.708353_QEAE
    • 2023-03-24_02:38:58.708358_ZPrE
    • 2023-03-24_02:38:58.708363_CrqE
    • 2023-03-24_02:38:58.708368_oOno
    redfin-30
    Percentile687e079926d683DiffSignificant (?)
    p10232 ±4 ms253 ±2 ms+21.2 ms (+9.1%)MAYBE
    p25238 ±6 ms258 ±2 ms+20.1 ms (+8.4%)MAYBE
    p50246 ±7 ms266 ±3 ms+19.9 ms (+8.1%)MAYBE
    p75255 ±7 ms276 ±4 ms+21.1 ms (+8.3%)MAYBE
    p90264 ±8 ms291 ±9 ms+26.6 ms (+10.0%)MAYBE

    20 test runs in comparison
    CommitTest Runs
    687e079
    • 2023-03-23_16:39:37.443178_SHcH
    • 2023-03-23_16:39:37.445852_XCTk
    • 2023-03-23_16:39:37.445868_fJBT
    • 2023-03-23_16:39:37.445873_WBqI
    • 2023-03-23_16:39:37.445877_wdTg
    • 2023-03-23_16:39:37.445895_SLKB
    • 2023-03-23_16:39:37.445900_vfIH
    • 2023-03-23_16:39:37.445904_igIy
    • 2023-03-23_16:39:37.445908_KIUG
    • 2023-03-23_16:39:37.445912_EQdm
    926d683
    • 2023-03-24_02:38:58.707172_DhNU
    • 2023-03-24_02:38:58.708320_Flao
    • 2023-03-24_02:38:58.708331_qOuv
    • 2023-03-24_02:38:58.708337_Lhpp
    • 2023-03-24_02:38:58.708342_igkY
    • 2023-03-24_02:38:58.708347_dAPx
    • 2023-03-24_02:38:58.708353_QEAE
    • 2023-03-24_02:38:58.708358_ZPrE
    • 2023-03-24_02:38:58.708363_CrqE
    • 2023-03-24_02:38:58.708368_oOno

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

@dconeybe dconeybe requested a review from milaGGL March 23, 2023 14:54
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

@dconeybe dconeybe requested a review from milaGGL March 24, 2023 01:30
@dconeybe dconeybe merged commit 3d4cfed into master Mar 24, 2023
@dconeybe dconeybe deleted the dconeybe/QueryExistenceFilterTest2 branch March 24, 2023 03:57
@firebase firebase locked and limited conversation to collaborators Apr 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants