Skip to content
This repository was archived by the owner on Mar 4, 2026. It is now read-only.

fix: fix flaky BulkWriter rate limiter test#1307

Merged
thebrianchen merged 3 commits intomasterfrom
bc/bulk-flake
Sep 25, 2020
Merged

fix: fix flaky BulkWriter rate limiter test#1307
thebrianchen merged 3 commits intomasterfrom
bc/bulk-flake

Conversation

@thebrianchen
Copy link
Copy Markdown

The test flakes when the CI slows down, which allows enough time to pass such that the test's writes can be made without exceeding the rate limit. To fix this, I created a mock rate limiter that always rejects any request, which makes this test timing-agnostic.

@thebrianchen thebrianchen requested review from a team September 23, 2020 15:11
@thebrianchen thebrianchen self-assigned this Sep 23, 2020
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 23, 2020
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 23, 2020

Codecov Report

Merging #1307 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1307   +/-   ##
=======================================
  Coverage   98.51%   98.51%           
=======================================
  Files          30       30           
  Lines       18754    18760    +6     
  Branches     1450     1451    +1     
=======================================
+ Hits        18475    18481    +6     
  Misses        276      276           
  Partials        3        3           
Impacted Files Coverage Δ
dev/src/bulk-writer.ts 99.76% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57dcf1c...5377d40. Read the comment docs.

@product-auto-label product-auto-label Bot added the api: firestore Issues related to the googleapis/nodejs-firestore API. label Sep 24, 2020
@thebrianchen
Copy link
Copy Markdown
Author

Update: also added an additional check in the BulkWriter constructor to lower the max batch size to match the starting rate if the starting rate is lower than the max batch size.

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

Labels

api: firestore Issues related to the googleapis/nodejs-firestore API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants