Firestore: Update test bloomFilterShouldCorrectlyEncodeComplexUnicodeCharacters() in QueryTest.java #5145

dconeybe · 2023-07-07T03:14:18Z

Modify the test bloomFilterShouldCorrectlyEncodeComplexUnicodeCharacters() in QueryTest.java, that was added by the recent PR #5135, to directly check the bloom filter for containing the document paths with complex Unicode characters. Previously, it was indirectly checking this via the other properties of the existence filter. As a result, the "retry" logic was removed, since it doesn't matter if there is a false positive.

To add the ability to directly interrogate the bloom filter, the testing hooks needed to be augmented to expose the BloomFilter, projectId, and databaseId.

This is a port of firebase/firebase-js-sdk#7413.

Some refactorings/cleanup from this PR were backported to the web sdk in firebase/firebase-js-sdk#7474.

…ing the document paths with complex Unicode characters, rather than indirectly testing it via the QuerySnapshot.

github-actions · 2023-07-07T03:14:32Z

Release note changes

No release note changes were detected. If you made changes that should be
present in the next release, ensure you've added an entry in the appropriate
CHANGELOG.md file(s).

google-oss-bot · 2023-07-07T03:20:34Z

Coverage Report ¹

Affected Products

`firebase-firestore`

Overall coverage changed from 44.35% (4d1cc39) to 44.33% (05fe38b) by -0.02%.

Filename	Base (`4d1cc39`)	Merge (`05fe38b`)	Diff
AutoValue_TestingHooks_ExistenceFilterMismatchInfo.java	20.00%	23.08%	+3.08%
LruGarbageCollector.java	97.27%	93.64%	-3.64%
PatchMutation.java	100.00%	98.39%	-1.61%
TestingHooks.java	64.52%	66.67%	+2.15%
WatchChangeAggregator.java	98.60%	98.64%	+0.03%

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/4okmtvMvZv.html

github-actions · 2023-07-07T03:24:36Z

Unit Test Results

  162 files +  108   162 suites +108 2m 7s ⏱️ - 4m 58s
1 164 tests +  694 1 148 ✔️ +  678 16 💤 +16 0 ❌ ±0
2 328 runs +1 388 2 296 ✔️ +1 356 32 💤 +32 0 ❌ ±0

Results for commit 41e84f3. ± Comparison against base commit 4d1cc39.

This pull request removes 470 and adds 1164 tests. Note that renamed tests count towards both.

com.google.firebase.iid.FirebaseInstanceIdWithFcmReceiverRoboTest ‑ testBindsService_oAndTargetingO
com.google.firebase.iid.FirebaseInstanceIdWithFcmReceiverRoboTest ‑ testNoWrappedIntent
com.google.firebase.iid.FirebaseInstanceIdWithFcmReceiverRoboTest ‑ testNullIntent
com.google.firebase.iid.FirebaseInstanceIdWithFcmReceiverRoboTest ‑ testStartsService_OTargetingO_highPriority
com.google.firebase.iid.FirebaseInstanceIdWithFcmReceiverRoboTest ‑ testStartsService_fallsBackToBindService
com.google.firebase.iid.FirebaseInstanceIdWithFcmReceiverRoboTest ‑ testStartsService_notOButTargetingO
com.google.firebase.iid.FirebaseInstanceIdWithFcmReceiverRoboTest ‑ testStartsService_notOButTargetingO[19]
com.google.firebase.iid.FirebaseInstanceIdWithFcmReceiverRoboTest ‑ testStartsService_notOButTargetingO[21]
com.google.firebase.iid.FirebaseInstanceIdWithFcmReceiverRoboTest ‑ testStartsService_notOButTargetingO[22]
com.google.firebase.iid.FirebaseInstanceIdWithFcmReceiverRoboTest ‑ testStartsService_notOButTargetingO[23]
…

com.google.firebase.TimestampTest ‑ testCompare
com.google.firebase.TimestampTest ‑ testFromDate
com.google.firebase.TimestampTest ‑ testRejectBadDates
com.google.firebase.TimestampTest ‑ testTimestampParcelable
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
…

♻️ This comment has been updated with latest results.

google-oss-bot · 2023-07-07T03:28:15Z

Size Report ¹

Affected Products

firebase-firestore
Type Base (4d1cc39) Merge (05fe38b) Diff
aar 1.36 MB 1.36 MB +908 B (+0.1%)
apk (release) 3.95 MB 3.95 MB +668 B (+0.0%)

Test Logs

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

google-oss-bot · 2023-07-07T03:51:13Z

Startup Time Report ¹

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

Notes

This report is for comparing the base commit (4d1cc39) and the CI merge commit (05fe38b)
Please check below reports for each individual commit to find more details (Perfetto traces, histograms, detailed measurements)
- 4d1cc39: https://storage.googleapis.com/firebase-sdk-metric-reports/APx6JfxDsH/index.html
- 05fe38b: https://storage.googleapis.com/firebase-sdk-metric-reports/t18ZXG57S9/index.html

Startup Times

`fire-fst`

Device Statistics Distributions

oriole-32

Percentile	`4d1cc39`	`05fe38b`	Diff	Significant (?)
p10	325 ±19 μs	325 ±9 μs	-571 ns (-0.2%)	NO
p25	340 ±21 μs	337 ±14 μs	-2.50 μs (-0.7%)	NO
p50	360 ±32 μs	365 ±34 μs	+4.67 μs (+1.3%)	NO
p75	395 ±59 μs	434 ±82 μs	+38.3 μs (+9.7%)	NO
p90	452 ±117 μs	572 ±178 μs	+120 μs (+26.4%)	NO

20 test runs in comparison

Commit	Test Runs
`4d1cc39`	2023-07-11_20:46:15.787298_iVgi 2023-07-11_20:46:15.790597_mFVb 2023-07-11_20:46:15.790617_vqYw 2023-07-11_20:46:15.790625_PWnY 2023-07-11_20:46:15.790633_aYqd 2023-07-11_20:46:15.790732_vLrc 2023-07-11_20:46:15.790741_eGlR 2023-07-11_20:46:15.790747_dLZh 2023-07-11_20:46:15.790754_cdsX 2023-07-11_20:46:15.790760_gpFg
`05fe38b`	2023-07-13_15:04:39.419713_paAS 2023-07-13_15:04:39.426916_scub 2023-07-13_15:04:39.426927_mQhG 2023-07-13_15:04:39.427063_DZav 2023-07-13_15:04:39.427069_LZMj 2023-07-13_15:04:39.427075_RDnZ 2023-07-13_15:04:39.427081_JnDe 2023-07-13_15:04:39.427086_PATe 2023-07-13_15:04:39.427092_xWOs 2023-07-13_15:04:39.427098_ilui

redfin-30

Percentile	`4d1cc39`	`05fe38b`	Diff	Significant (?)
p10	607 ±26 μs	626 ±28 μs	+19.4 μs (+3.2%)	NO
p25	623 ±33 μs	645 ±31 μs	+22.0 μs (+3.5%)	NO
p50	649 ±39 μs	670 ±32 μs	+21.7 μs (+3.3%)	NO
p75	691 ±56 μs	706 ±33 μs	+14.8 μs (+2.1%)	NO
p90	749 ±85 μs	752 ±44 μs	+3.01 μs (+0.4%)	NO

20 test runs in comparison

Commit	Test Runs
`4d1cc39`	2023-07-11_20:46:15.787298_iVgi 2023-07-11_20:46:15.790597_mFVb 2023-07-11_20:46:15.790617_vqYw 2023-07-11_20:46:15.790625_PWnY 2023-07-11_20:46:15.790633_aYqd 2023-07-11_20:46:15.790732_vLrc 2023-07-11_20:46:15.790741_eGlR 2023-07-11_20:46:15.790747_dLZh 2023-07-11_20:46:15.790754_cdsX 2023-07-11_20:46:15.790760_gpFg
`05fe38b`	2023-07-13_15:04:39.419713_paAS 2023-07-13_15:04:39.426916_scub 2023-07-13_15:04:39.426927_mQhG 2023-07-13_15:04:39.427063_DZav 2023-07-13_15:04:39.427069_LZMj 2023-07-13_15:04:39.427075_RDnZ 2023-07-13_15:04:39.427081_JnDe 2023-07-13_15:04:39.427086_PATe 2023-07-13_15:04:39.427092_xWOs 2023-07-13_15:04:39.427098_ilui

`timeToInitialDisplay`

Device Statistics Distributions

oriole-32

Percentile	`4d1cc39`	`05fe38b`	Diff	Significant (?)
p10	202 ±7 ms	206 ±3 ms	+3.44 ms (+1.7%)	NO
p25	209 ±8 ms	211 ±4 ms	+2.08 ms (+1.0%)	NO
p50	217 ±8 ms	220 ±4 ms	+2.75 ms (+1.3%)	NO
p75	226 ±8 ms	229 ±4 ms	+3.58 ms (+1.6%)	NO
p90	233 ±7 ms	242 ±7 ms	+9.13 ms (+3.9%)	NO

20 test runs in comparison

Commit	Test Runs
`4d1cc39`	2023-07-11_20:46:15.787298_iVgi 2023-07-11_20:46:15.790597_mFVb 2023-07-11_20:46:15.790617_vqYw 2023-07-11_20:46:15.790625_PWnY 2023-07-11_20:46:15.790633_aYqd 2023-07-11_20:46:15.790732_vLrc 2023-07-11_20:46:15.790741_eGlR 2023-07-11_20:46:15.790747_dLZh 2023-07-11_20:46:15.790754_cdsX 2023-07-11_20:46:15.790760_gpFg
`05fe38b`	2023-07-13_15:04:39.419713_paAS 2023-07-13_15:04:39.426916_scub 2023-07-13_15:04:39.426927_mQhG 2023-07-13_15:04:39.427063_DZav 2023-07-13_15:04:39.427069_LZMj 2023-07-13_15:04:39.427075_RDnZ 2023-07-13_15:04:39.427081_JnDe 2023-07-13_15:04:39.427086_PATe 2023-07-13_15:04:39.427092_xWOs 2023-07-13_15:04:39.427098_ilui

redfin-30

Percentile	`4d1cc39`	`05fe38b`	Diff	Significant (?)
p10	242 ±3 ms	271 ±6 ms	+29.2 ms (+12.1%)	YES
p25	248 ±3 ms	278 ±6 ms	+30.0 ms (+12.1%)	YES
p50	255 ±3 ms	287 ±6 ms	+31.5 ms (+12.3%)	YES
p75	264 ±3 ms	297 ±7 ms	+33.5 ms (+12.7%)	YES
p90	273 ±4 ms	313 ±11 ms	+40.0 ms (+14.6%)	MAYBE

20 test runs in comparison

Commit	Test Runs
`4d1cc39`	2023-07-11_20:46:15.787298_iVgi 2023-07-11_20:46:15.790597_mFVb 2023-07-11_20:46:15.790617_vqYw 2023-07-11_20:46:15.790625_PWnY 2023-07-11_20:46:15.790633_aYqd 2023-07-11_20:46:15.790732_vLrc 2023-07-11_20:46:15.790741_eGlR 2023-07-11_20:46:15.790747_dLZh 2023-07-11_20:46:15.790754_cdsX 2023-07-11_20:46:15.790760_gpFg
`05fe38b`	2023-07-13_15:04:39.419713_paAS 2023-07-13_15:04:39.426916_scub 2023-07-13_15:04:39.426927_mQhG 2023-07-13_15:04:39.427063_DZav 2023-07-13_15:04:39.427069_LZMj 2023-07-13_15:04:39.427075_RDnZ 2023-07-13_15:04:39.427081_JnDe 2023-07-13_15:04:39.427086_PATe 2023-07-13_15:04:39.427092_xWOs 2023-07-13_15:04:39.427098_ilui

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

milaGGL

LGTM

Firestore: QueryTest.java: Directly test the bloom filter for contain…

8164144

…ing the document paths with complex Unicode characters, rather than indirectly testing it via the QuerySnapshot.

dconeybe added api: firestore no-changelog main-merge-ack labels Jul 7, 2023

dconeybe self-assigned this Jul 7, 2023

google-oss-bot added the do-not-merge/work-in-progress label Jul 7, 2023

google-oss-bot added the size/L label Jul 7, 2023

dconeybe marked this pull request as ready for review July 7, 2023 03:53

google-oss-bot removed the do-not-merge/work-in-progress label Jul 7, 2023

dconeybe requested a review from milaGGL July 7, 2023 03:53

Merge branch 'master' into dconeybe/BloomFilterUnicodeTestSimplification

41e84f3

milaGGL approved these changes Jul 14, 2023

View reviewed changes

dconeybe merged commit 800d290 into master Jul 14, 2023

dconeybe deleted the dconeybe/BloomFilterUnicodeTestSimplification branch July 14, 2023 16:44

This was referenced Jul 14, 2023

Update test 'bloom filter should correctly encode special unicode characters' in query.test.ts firebase/firebase-js-sdk#7413

Merged

Firestore: Misc bloom filter cleanup ported from Android firebase/firebase-js-sdk#7474

Merged

firebase locked and limited conversation to collaborators Aug 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Firestore: Update test bloomFilterShouldCorrectlyEncodeComplexUnicodeCharacters() in QueryTest.java #5145

Firestore: Update test bloomFilterShouldCorrectlyEncodeComplexUnicodeCharacters() in QueryTest.java #5145

dconeybe commented Jul 7, 2023 •

edited

Loading

github-actions bot commented Jul 7, 2023 •

edited

Loading

google-oss-bot commented Jul 7, 2023 •

edited

Loading

`firebase-firestore`

github-actions bot commented Jul 7, 2023 •

edited

Loading

google-oss-bot commented Jul 7, 2023 •

edited

Loading

`firebase-firestore`

google-oss-bot commented Jul 7, 2023 •

edited

Loading

`fire-fst`

`timeToInitialDisplay`

milaGGL left a comment

Firestore: Update test bloomFilterShouldCorrectlyEncodeComplexUnicodeCharacters() in QueryTest.java #5145

Firestore: Update test bloomFilterShouldCorrectlyEncodeComplexUnicodeCharacters() in QueryTest.java #5145

Conversation

dconeybe commented Jul 7, 2023 • edited Loading

github-actions bot commented Jul 7, 2023 • edited Loading

Release note changes

google-oss-bot commented Jul 7, 2023 • edited Loading

Coverage Report 1

Affected Products

firebase-firestore

Test Logs

github-actions bot commented Jul 7, 2023 • edited Loading

Unit Test Results

google-oss-bot commented Jul 7, 2023 • edited Loading

Size Report 1

Affected Products

firebase-firestore

Test Logs

google-oss-bot commented Jul 7, 2023 • edited Loading

Startup Time Report 1

Notes

Startup Times

fire-fst

timeToInitialDisplay

milaGGL left a comment

Choose a reason for hiding this comment

dconeybe commented Jul 7, 2023 •

edited

Loading

github-actions bot commented Jul 7, 2023 •

edited

Loading

google-oss-bot commented Jul 7, 2023 •

edited

Loading

Coverage Report ¹

`firebase-firestore`

github-actions bot commented Jul 7, 2023 •

edited

Loading

google-oss-bot commented Jul 7, 2023 •

edited

Loading

Size Report ¹

`firebase-firestore`

google-oss-bot commented Jul 7, 2023 •

edited

Loading

Startup Time Report ¹

`fire-fst`

`timeToInitialDisplay`