Skip to content

test: fix flaky testBucketPolicyV1RequesterPays it test#379

Merged
frankyn merged 3 commits intogoogleapis:masterfrom
dmitry-fa:falky
Jun 17, 2020
Merged

test: fix flaky testBucketPolicyV1RequesterPays it test#379
frankyn merged 3 commits intogoogleapis:masterfrom
dmitry-fa:falky

Conversation

@dmitry-fa
Copy link
Copy Markdown
Contributor

The test is updated to use a new bucket (and delete it at the end) instead of reusing the shared one.

Fixes #349

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 17, 2020
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 17, 2020

Codecov Report

Merging #379 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #379   +/-   ##
=========================================
  Coverage     63.13%   63.13%           
  Complexity      601      601           
=========================================
  Files            32       32           
  Lines          5078     5078           
  Branches        481      481           
=========================================
  Hits           3206     3206           
  Misses         1708     1708           
  Partials        164      164           

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 5d33d64...b198650. Read the comment docs.

@dmitry-fa dmitry-fa requested review from elharo and frankyn June 17, 2020 12:12
storage.get(
bucketName, Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
assertNull(remoteBucket.requesterPays());
remoteBucket = remoteBucket.toBuilder().setRequesterPays(true).build();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try not to reuse local variables for different objects as it's very error prone when code is updated.

ImmutableList.of("storage.buckets.getIamPolicy", "storage.buckets.setIamPolicy"),
bucketOptions));
remoteBucket = remoteBucket.toBuilder().setRequesterPays(false).build();
updatedBucket =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try not to reuse local variables as it's very error prone when code is updated.

assertTrue(updatedBucket.requesterPays());
assertNull(bucketDefault.requesterPays());

Bucket bucketTrue = storage.update(bucketDefault.toBuilder().setRequesterPays(true).build());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

requesterPaysBucket? modifiedBucket?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To me bucketDefault, bucketTrue, bucketFalse sound more informative than bucket, modifiedBucket, modifiedBucket2. But I'm okay with any names.

@frankyn frankyn merged commit 0868cd4 into googleapis:master Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

storage.it.ITStorageTest: testDownloadPublicBlobWithoutAuthentication failed

4 participants