Repository: googleapis/java-storage
Pull request #439 (2020-07-20 18:43:45)

test: ITStorage cleanup
The following changes have been done:

- `testListBlobRequesterPays`, `testUpdateBucketRequesterPays` - make independent from other tests
- `testAttemptDeletionObjectTemporaryHold` - restoring operation corrected
- `testBucketAcl`, `testUpdateBucketRequesterPays` , `testBucketPolicyV1RequesterPays` - updated to use the dedicated bucket
- @Before code now is invoked only by a few tests, not all

Fixes: #409


codecov[bot]: # [Codecov](https://codecov.io/gh/googleapis/java-storage/pull/439?src=pr&el=h1) Report
> Merging [#439](https://codecov.io/gh/googleapis/java-storage/pull/439?src=pr&el=desc) into [master](https://codecov.io/gh/googleapis/java-storage/commit/e63feb7c684b9f5d4c9ec18edb33172b22bd01f0&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/googleapis/java-storage/pull/439/graphs/tree.svg?width=650&height=150&src=pr&token=ssm139YOdp)](https://codecov.io/gh/googleapis/java-storage/pull/439?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             master     #439   +/-   ##
=========================================
  Coverage     63.15%   63.15%           
- Complexity      610      619    +9     
=========================================
  Files            32       32           
  Lines          5133     5133           
  Branches        490      489    -1     
=========================================
  Hits           3242     3242           
  Misses         1726     1726           
  Partials        165      165           
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/googleapis/java-storage/pull/439?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/googleapis/java-storage/pull/439?src=pr&el=footer). Last update [e63feb7...dc47720](https://codecov.io/gh/googleapis/java-storage/pull/439?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

dmitry-fa: Generic question. I wrote a utility to run IT cases individually. With a very small change, it could be applied not to ITStorageTest but to any IT test based on JUnit. Not a masterpiece, but could be helpful in the future. Does it make sense to integrate this somewhere:

```
package com.google.cloud.storage.it;

import java.io.IOException;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.junit.Test;
import org.junit.runner.JUnitCore;
import org.junit.runner.Request;
import org.junit.runner.Result;

/**
 * Utility class to run all IT Storage test cases individually. This class extracts the existing
 * case names from {@link ITStorageTest} and launches a new JVM for each case.
 *
 * <p>Running tests individually allows to detect tests which rely on the effect from
 * running other tests.
 *
 * <p>To run this utility just compile and run this class with the same classpath as you run
 * {@link ITStorageTest}. Two environment variables are required to be set:
 * {@code GOOGLE_APPLICATION_CREDENTIALS} and  {@code IT_SERVICE_ACCOUNT_EMAIL}
 */
public class RunIndividually {

  static final Class IT_CLASS = ITStorageTest.class;

  static final List<String> MANDATORY =
      Arrays.asList("GOOGLE_APPLICATION_CREDENTIALS", "IT_SERVICE_ACCOUNT_EMAIL");

  public static void main(String... args) throws InterruptedException, IOException {
    for (String var : MANDATORY) {
      if (System.getenv(var) == null) {
        System.err.println("Mandatory environment variable '" + var + "' is not set.");
        System.exit(1);
      }
    }

    if (args.length == 0) {
      String separator = System.getProperty("file.separator");
      String classpath = System.getProperty("java.class.path");
      String path = System.getProperty("java.home") + separator + "bin" + separator + "java";

      List<String> tests = getTestList();
      List<String> failed = new ArrayList<>();
      for (int i = 0; i < tests.size(); i++) {
        String test = tests.get(i);
        System.out.println("#" + (i + 1) + " of " + tests.size());
        ProcessBuilder processBuilder =
            new ProcessBuilder(path, "-cp", classpath, RunIndividually.class.getName(), test)
                .redirectOutput(ProcessBuilder.Redirect.INHERIT)
                .redirectError(ProcessBuilder.Redirect.INHERIT);
        int exitCode = processBuilder.start().waitFor();
        if (exitCode != 0) {
          failed.add(test);
        }
      }
      System.out.println(failed);
      System.out.println("Total failed " + failed.size());
    } else {
      JUnitCore core = new JUnitCore();
      for (String test : args) {
        System.out.println("----- " + test + " ------ ");
        Request request = Request.method(IT_CLASS, test);
        Result result = core.run(request);
        System.out.println("### " + test + " --> " + result.wasSuccessful());
        if (!result.wasSuccessful()) {
          System.out.println(result.getFailures().get(0).getMessage());
          System.exit(97);
        }
      }
    }
  }

  static List<String> getTestList() {
    List<String> tests = new ArrayList<>();
    for (Method method : IT_CLASS.getDeclaredMethods()) {
      if (method.isAnnotationPresent(Test.class)) {
        tests.add(method.getName());
      }
    }
    return tests;
  }
}

```
elharo: Why do you think the test methods are connected? Where is the shared state?
dmitry-fa: @elharo 
> Why do you think the test methods are connected? Where is the shared state?

Test methods should not be connected, but sometimes there are. I found two storage tests that checked that a bucket property set to `false`, they passed when were run in the bundle because some other tests set this property to `false`. Being run individually they failed because that property was set to `null`.

There could be other non-trivial dependencies between tests which not easy to catch. They could start failing unexpectedly when the test execution order changes.

elharo: @dmitry-fa This is a real YAGNI. If the test methods are coupled, that's what needs to be fixed. Starting up a new VM for each test is way too heavyweight and still won't address the most likely case of test coupling here: shared tests resources outside the VM in the filesystem or GCS. 
dmitry-fa: @elharo The suggested utility is not a replacement of the current test execution, it's a tool to discover the coupled tests. It could be run from time to time. And my question does it make sense to integrate it somewhere?

Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 39d1c88d..f2dbdee0 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -48,7 +48,6 @@ integration tests.
 
 ```bash
 export GOOGLE_APPLICATION_CREDENTIALS=/path/to/service/account.json
-export IT_SERVICE_ACCOUNT_EMAIL=my-service-account@my-project.gserviceaccount.com
 mvn -Penable-integration-tests clean verify
 ```
 
diff --git a/README.md b/README.md
index 230e2307..000f72b3 100644
--- a/README.md
+++ b/README.md
@@ -17,7 +17,7 @@ If you are using Maven with [BOM][libraries-bom], add this to your pom.xml file
     <dependency>
       <groupId>com.google.cloud</groupId>
       <artifactId>libraries-bom</artifactId>
-      <version>7.0.0</version>
+      <version>8.1.0</version>
       <type>pom</type>
       <scope>import</scope>
     </dependency>
@@ -38,7 +38,7 @@ If you are using Maven without BOM, add this to your dependencies:
 <dependency>
   <groupId>com.google.cloud</groupId>
   <artifactId>google-cloud-storage</artifactId>
-  <version>1.111.0</version>
+  <version>1.111.2</version>
 </dependency>
 
 ```
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
index 9a716d70..36136363 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
@@ -1240,7 +1240,7 @@ public class BucketInfo implements Serializable {
 
     @Override
     public Builder setCors(Iterable<Cors> cors) {
-      this.cors = cors != null ? ImmutableList.copyOf(cors) : null;
+      this.cors = cors != null ? ImmutableList.copyOf(cors) : ImmutableList.<Cors>of();
       return this;
     }
 
diff --git a/synth.metadata b/synth.metadata
index ed7f3e53..f19c30e8 100644
--- a/synth.metadata
+++ b/synth.metadata
@@ -4,7 +4,7 @@
       "git": {
         "name": ".",
         "remote": "https://github.com/googleapis/java-storage.git",
-        "sha": "a5773d4c43127e808ee56b7185e58af6c3cdb8a2"
+        "sha": "f8a4b12517c661881d7b7c65f796c1c8f1cf3ae9"
       }
     },
     {
@@ -14,5 +14,81 @@
         "sha": "4f2c9f752a94042472fc03c5bd9e06e89817d2bd"
       }
     }
+  ],
+  "generatedFiles": [
+    ".github/CODEOWNERS",
+    ".github/ISSUE_TEMPLATE/bug_report.md",
+    ".github/ISSUE_TEMPLATE/feature_request.md",
+    ".github/ISSUE_TEMPLATE/support_request.md",
+    ".github/PULL_REQUEST_TEMPLATE.md",
+    ".github/release-please.yml",
+    ".github/trusted-contribution.yml",
+    ".github/workflows/ci.yaml",
+    ".kokoro/build.bat",
+    ".kokoro/build.sh",
+    ".kokoro/coerce_logs.sh",
+    ".kokoro/common.cfg",
+    ".kokoro/common.sh",
+    ".kokoro/continuous/common.cfg",
+    ".kokoro/continuous/dependencies.cfg",
+    ".kokoro/continuous/integration.cfg",
+    ".kokoro/continuous/java11.cfg",
+    ".kokoro/continuous/java7.cfg",
+    ".kokoro/continuous/java8-osx.cfg",
+    ".kokoro/continuous/java8-win.cfg",
+    ".kokoro/continuous/java8.cfg",
+    ".kokoro/continuous/lint.cfg",
+    ".kokoro/continuous/propose_release.cfg",
+    ".kokoro/continuous/samples.cfg",
+    ".kokoro/dependencies.sh",
+    ".kokoro/linkage-monitor.sh",
+    ".kokoro/nightly/common.cfg",
+    ".kokoro/nightly/dependencies.cfg",
+    ".kokoro/nightly/integration.cfg",
+    ".kokoro/nightly/java11.cfg",
+    ".kokoro/nightly/java7.cfg",
+    ".kokoro/nightly/java8-osx.cfg",
+    ".kokoro/nightly/java8-win.cfg",
+    ".kokoro/nightly/java8.cfg",
+    ".kokoro/nightly/lint.cfg",
+    ".kokoro/nightly/samples.cfg",
+    ".kokoro/populate-secrets.sh",
+    ".kokoro/presubmit/clirr.cfg",
+    ".kokoro/presubmit/common.cfg",
+    ".kokoro/presubmit/dependencies.cfg",
+    ".kokoro/presubmit/java11.cfg",
+    ".kokoro/presubmit/java7.cfg",
+    ".kokoro/presubmit/java8-osx.cfg",
+    ".kokoro/presubmit/java8-win.cfg",
+    ".kokoro/presubmit/java8.cfg",
+    ".kokoro/presubmit/linkage-monitor.cfg",
+    ".kokoro/presubmit/lint.cfg",
+    ".kokoro/presubmit/samples.cfg",
+    ".kokoro/release/bump_snapshot.cfg",
+    ".kokoro/release/common.cfg",
+    ".kokoro/release/common.sh",
+    ".kokoro/release/drop.cfg",
+    ".kokoro/release/drop.sh",
+    ".kokoro/release/promote.cfg",
+    ".kokoro/release/promote.sh",
+    ".kokoro/release/publish_javadoc.cfg",
+    ".kokoro/release/publish_javadoc.sh",
+    ".kokoro/release/snapshot.cfg",
+    ".kokoro/release/snapshot.sh",
+    ".kokoro/release/stage.cfg",
+    ".kokoro/release/stage.sh",
+    ".kokoro/trampoline.sh",
+    "CODE_OF_CONDUCT.md",
+    "CONTRIBUTING.md",
+    "LICENSE",
+    "README.md",
+    "codecov.yaml",
+    "java.header",
+    "license-checks.xml",
+    "renovate.json",
+    "samples/install-without-bom/pom.xml",
+    "samples/pom.xml",
+    "samples/snapshot/pom.xml",
+    "samples/snippets/pom.xml"
   ]
 }
\ No newline at end of file


Test patch:
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java
index 94050ffe..862fbb35 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java
@@ -886,4 +886,36 @@ public class BucketTest {
     assertThat(actualUpdatedBucket.getLogging().getLogBucket()).isNull();
     assertThat(actualUpdatedBucket.getLogging().getLogObjectPrefix()).isNull();
   }
+
+  @Test
+  public void testRemoveBucketCORS() {
+    initializeExpectedBucket(6);
+    List<Cors.Origin> origins = ImmutableList.of(Cors.Origin.of("http://cloud.google.com"));
+    List<HttpMethod> httpMethods = ImmutableList.of(HttpMethod.GET);
+    List<String> responseHeaders = ImmutableList.of("Content-Type");
+    Cors cors =
+        Cors.newBuilder()
+            .setOrigins(origins)
+            .setMethods(httpMethods)
+            .setResponseHeaders(responseHeaders)
+            .setMaxAgeSeconds(100)
+            .build();
+    BucketInfo bucketInfo = BucketInfo.newBuilder("b").setCors(ImmutableList.of(cors)).build();
+    Bucket bucket = new Bucket(serviceMockReturnsOptions, new BucketInfo.BuilderImpl(bucketInfo));
+    assertThat(bucket.getCors()).isNotNull();
+    assertThat(bucket.getCors().get(0).getMaxAgeSeconds()).isEqualTo(100);
+    assertThat(bucket.getCors().get(0).getMethods()).isEqualTo(httpMethods);
+    assertThat(bucket.getCors().get(0).getOrigins()).isEqualTo(origins);
+    assertThat(bucket.getCors().get(0).getResponseHeaders()).isEqualTo(responseHeaders);
+
+    // Remove bucket CORS configuration.
+    Bucket expectedUpdatedBucket = bucket.toBuilder().setCors(null).build();
+    expect(storage.getOptions()).andReturn(mockOptions).times(2);
+    expect(storage.update(expectedUpdatedBucket)).andReturn(expectedUpdatedBucket);
+    replay(storage);
+    initializeBucket();
+    Bucket updatedBucket = new Bucket(storage, new BucketInfo.BuilderImpl(expectedUpdatedBucket));
+    Bucket actualUpdatedBucket = updatedBucket.update();
+    assertThat(actualUpdatedBucket.getCors()).isEmpty();
+  }
 }
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java
index 0f509271..59000a07 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java
@@ -66,6 +66,7 @@ import com.google.cloud.storage.BucketInfo.LifecycleRule;
 import com.google.cloud.storage.BucketInfo.LifecycleRule.LifecycleAction;
 import com.google.cloud.storage.BucketInfo.LifecycleRule.LifecycleCondition;
 import com.google.cloud.storage.CopyWriter;
+import com.google.cloud.storage.Cors;
 import com.google.cloud.storage.HmacKey;
 import com.google.cloud.storage.HttpMethod;
 import com.google.cloud.storage.PostPolicyV4;
@@ -135,7 +136,6 @@ import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.client.HttpClients;
 import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
 import org.junit.AfterClass;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -150,6 +150,7 @@ public class ITStorageTest {
       Metadata.Key.of("x-goog-request-params", Metadata.ASCII_STRING_MARSHALLER);
   private static final Logger log = Logger.getLogger(ITStorageTest.class.getName());
   private static final String BUCKET = RemoteStorageHelper.generateBucketName();
+  private static final String BUCKET_REQUESTER_PAYS = RemoteStorageHelper.generateBucketName();
   private static final String CONTENT_TYPE = "text/plain";
   private static final byte[] BLOB_BYTE_CONTENT = {0xD, 0xE, 0xA, 0xD};
   private static final String BLOB_STRING_CONTENT = "Hello Google Cloud Storage!";
@@ -212,15 +213,16 @@ public class ITStorageTest {
                         LifecycleCondition.newBuilder().setAge(1).build())))
             .build());
 
+    storage.create(BucketInfo.newBuilder(BUCKET_REQUESTER_PAYS).build());
+
     // Prepare KMS KeyRing for CMEK tests
     prepareKmsKeys();
   }
 
-  @Before
-  public void beforeEach() {
+  private static void unsetRequesterPays() {
     Bucket remoteBucket =
         storage.get(
-            BUCKET,
+            BUCKET_REQUESTER_PAYS,
             Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING),
             Storage.BucketGetOption.userProject(storage.getOptions().getProjectId()));
     // Disable requester pays in case a test fails to clean up.
@@ -246,6 +248,8 @@ public class ITStorageTest {
       if (!wasDeleted && log.isLoggable(Level.WARNING)) {
         log.log(Level.WARNING, "Deletion of bucket {0} timed out, bucket is not empty", BUCKET);
       }
+      unsetRequesterPays();
+      RemoteStorageHelper.forceDelete(storage, BUCKET_REQUESTER_PAYS, 5, TimeUnit.SECONDS);
     }
   }
 
@@ -912,22 +916,26 @@ public class ITStorageTest {
 
   @Test(timeout = 7500)
   public void testListBlobRequesterPays() throws InterruptedException {
+    unsetRequesterPays();
     BlobInfo blob1 =
-        BlobInfo.newBuilder(BUCKET, "test-list-blobs-empty-selected-fields-blob1")
+        BlobInfo.newBuilder(BUCKET_REQUESTER_PAYS, "test-list-blobs-empty-selected-fields-blob1")
             .setContentType(CONTENT_TYPE)
             .build();
     assertNotNull(storage.create(blob1));
 
     // Test listing a Requester Pays bucket.
     Bucket remoteBucket =
-        storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
-    assertFalse(remoteBucket.requesterPays());
+        storage.get(
+            BUCKET_REQUESTER_PAYS,
+            Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
+
+    assertTrue(remoteBucket.requesterPays() == null || !remoteBucket.requesterPays());
     remoteBucket = remoteBucket.toBuilder().setRequesterPays(true).build();
     Bucket updatedBucket = storage.update(remoteBucket);
     assertTrue(updatedBucket.requesterPays());
     try {
       storage.list(
-          BUCKET,
+          BUCKET_REQUESTER_PAYS,
           Storage.BlobListOption.prefix("test-list-blobs-empty-selected-fields-blob"),
           Storage.BlobListOption.fields(),
           Storage.BlobListOption.userProject("fakeBillingProjectId"));
@@ -940,7 +948,7 @@ public class ITStorageTest {
     while (true) {
       Page<Blob> page =
           storage.list(
-              BUCKET,
+              BUCKET_REQUESTER_PAYS,
               Storage.BlobListOption.prefix("test-list-blobs-empty-selected-fields-blob"),
               Storage.BlobListOption.fields(),
               Storage.BlobListOption.userProject(projectId));
@@ -2232,6 +2240,7 @@ public class ITStorageTest {
 
   @Test
   public void testBucketAcl() {
+    unsetRequesterPays();
     testBucketAclRequesterPays(true);
     testBucketAclRequesterPays(false);
   }
@@ -2239,8 +2248,10 @@ public class ITStorageTest {
   private void testBucketAclRequesterPays(boolean requesterPays) {
     if (requesterPays) {
       Bucket remoteBucket =
-          storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
-      assertNull(remoteBucket.requesterPays());
+          storage.get(
+              BUCKET_REQUESTER_PAYS,
+              Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
+      assertTrue(remoteBucket.requesterPays() == null || !remoteBucket.requesterPays());
       remoteBucket = remoteBucket.toBuilder().setRequesterPays(true).build();
       Bucket updatedBucket = storage.update(remoteBucket);
       assertTrue(updatedBucket.requesterPays());
@@ -2253,22 +2264,27 @@ public class ITStorageTest {
             ? new Storage.BucketSourceOption[] {Storage.BucketSourceOption.userProject(projectId)}
             : new Storage.BucketSourceOption[] {};
 
-    assertNull(storage.getAcl(BUCKET, User.ofAllAuthenticatedUsers(), bucketOptions));
-    assertFalse(storage.deleteAcl(BUCKET, User.ofAllAuthenticatedUsers(), bucketOptions));
+    assertNull(
+        storage.getAcl(BUCKET_REQUESTER_PAYS, User.ofAllAuthenticatedUsers(), bucketOptions));
+    assertFalse(
+        storage.deleteAcl(BUCKET_REQUESTER_PAYS, User.ofAllAuthenticatedUsers(), bucketOptions));
     Acl acl = Acl.of(User.ofAllAuthenticatedUsers(), Role.READER);
-    assertNotNull(storage.createAcl(BUCKET, acl, bucketOptions));
+    assertNotNull(storage.createAcl(BUCKET_REQUESTER_PAYS, acl, bucketOptions));
     Acl updatedAcl =
-        storage.updateAcl(BUCKET, acl.toBuilder().setRole(Role.WRITER).build(), bucketOptions);
+        storage.updateAcl(
+            BUCKET_REQUESTER_PAYS, acl.toBuilder().setRole(Role.WRITER).build(), bucketOptions);
     assertEquals(Role.WRITER, updatedAcl.getRole());
     Set<Acl> acls = new HashSet<>();
-    acls.addAll(storage.listAcls(BUCKET, bucketOptions));
+    acls.addAll(storage.listAcls(BUCKET_REQUESTER_PAYS, bucketOptions));
     assertTrue(acls.contains(updatedAcl));
-    assertTrue(storage.deleteAcl(BUCKET, User.ofAllAuthenticatedUsers(), bucketOptions));
-    assertNull(storage.getAcl(BUCKET, User.ofAllAuthenticatedUsers(), bucketOptions));
+    assertTrue(
+        storage.deleteAcl(BUCKET_REQUESTER_PAYS, User.ofAllAuthenticatedUsers(), bucketOptions));
+    assertNull(
+        storage.getAcl(BUCKET_REQUESTER_PAYS, User.ofAllAuthenticatedUsers(), bucketOptions));
     if (requesterPays) {
       Bucket remoteBucket =
           storage.get(
-              BUCKET,
+              BUCKET_REQUESTER_PAYS,
               Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING),
               Storage.BucketGetOption.userProject(projectId));
       assertTrue(remoteBucket.requesterPays());
@@ -2460,80 +2476,75 @@ public class ITStorageTest {
 
   @Test
   public void testBucketPolicyV1RequesterPays() throws ExecutionException, InterruptedException {
-    String bucketName = RemoteStorageHelper.generateBucketName();
-    storage.create(BucketInfo.newBuilder(bucketName).build());
+    unsetRequesterPays();
+    Bucket bucketDefault =
+        storage.get(
+            BUCKET_REQUESTER_PAYS,
+            Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
+    assertTrue(bucketDefault.requesterPays() == null || !bucketDefault.requesterPays());
 
-    try {
-      Bucket bucketDefault =
-          storage.get(
-              bucketName, Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
-      assertNull(bucketDefault.requesterPays());
-
-      Bucket bucketTrue = storage.update(bucketDefault.toBuilder().setRequesterPays(true).build());
-      assertTrue(bucketTrue.requesterPays());
-
-      String projectId = remoteStorageHelper.getOptions().getProjectId();
-
-      Storage.BucketSourceOption[] bucketOptions =
-          new Storage.BucketSourceOption[] {Storage.BucketSourceOption.userProject(projectId)};
-      Identity projectOwner = Identity.projectOwner(projectId);
-      Identity projectEditor = Identity.projectEditor(projectId);
-      Identity projectViewer = Identity.projectViewer(projectId);
-      Map<com.google.cloud.Role, Set<Identity>> bindingsWithoutPublicRead =
-          ImmutableMap.of(
-              StorageRoles.legacyBucketOwner(),
-              new HashSet<>(Arrays.asList(projectOwner, projectEditor)),
-              StorageRoles.legacyBucketReader(),
-              (Set<Identity>) new HashSet<>(Collections.singleton(projectViewer)));
-      Map<com.google.cloud.Role, Set<Identity>> bindingsWithPublicRead =
-          ImmutableMap.of(
-              StorageRoles.legacyBucketOwner(),
-              new HashSet<>(Arrays.asList(projectOwner, projectEditor)),
-              StorageRoles.legacyBucketReader(),
-              new HashSet<>(Collections.singleton(projectViewer)),
-              StorageRoles.legacyObjectReader(),
-              (Set<Identity>) new HashSet<>(Collections.singleton(Identity.allUsers())));
-
-      // Validate getting policy.
-      Policy currentPolicy = storage.getIamPolicy(bucketName, bucketOptions);
-      assertEquals(bindingsWithoutPublicRead, currentPolicy.getBindings());
-
-      // Validate updating policy.
-      Policy updatedPolicy =
-          storage.setIamPolicy(
-              bucketName,
-              currentPolicy
-                  .toBuilder()
-                  .addIdentity(StorageRoles.legacyObjectReader(), Identity.allUsers())
-                  .build(),
-              bucketOptions);
-      assertEquals(bindingsWithPublicRead, updatedPolicy.getBindings());
-      Policy revertedPolicy =
-          storage.setIamPolicy(
-              bucketName,
-              updatedPolicy
-                  .toBuilder()
-                  .removeIdentity(StorageRoles.legacyObjectReader(), Identity.allUsers())
-                  .build(),
-              bucketOptions);
-      assertEquals(bindingsWithoutPublicRead, revertedPolicy.getBindings());
-
-      // Validate testing permissions.
-      List<Boolean> expectedPermissions = ImmutableList.of(true, true);
-      assertEquals(
-          expectedPermissions,
-          storage.testIamPermissions(
-              bucketName,
-              ImmutableList.of("storage.buckets.getIamPolicy", "storage.buckets.setIamPolicy"),
-              bucketOptions));
-      Bucket bucketFalse =
-          storage.update(
-              bucketTrue.toBuilder().setRequesterPays(false).build(),
-              Storage.BucketTargetOption.userProject(projectId));
-      assertFalse(bucketFalse.requesterPays());
-    } finally {
-      RemoteStorageHelper.forceDelete(storage, bucketName, 5, TimeUnit.SECONDS);
-    }
+    Bucket bucketTrue = storage.update(bucketDefault.toBuilder().setRequesterPays(true).build());
+    assertTrue(bucketTrue.requesterPays());
+
+    String projectId = remoteStorageHelper.getOptions().getProjectId();
+
+    Storage.BucketSourceOption[] bucketOptions =
+        new Storage.BucketSourceOption[] {Storage.BucketSourceOption.userProject(projectId)};
+    Identity projectOwner = Identity.projectOwner(projectId);
+    Identity projectEditor = Identity.projectEditor(projectId);
+    Identity projectViewer = Identity.projectViewer(projectId);
+    Map<com.google.cloud.Role, Set<Identity>> bindingsWithoutPublicRead =
+        ImmutableMap.of(
+            StorageRoles.legacyBucketOwner(),
+            new HashSet<>(Arrays.asList(projectOwner, projectEditor)),
+            StorageRoles.legacyBucketReader(),
+            (Set<Identity>) new HashSet<>(Collections.singleton(projectViewer)));
+    Map<com.google.cloud.Role, Set<Identity>> bindingsWithPublicRead =
+        ImmutableMap.of(
+            StorageRoles.legacyBucketOwner(),
+            new HashSet<>(Arrays.asList(projectOwner, projectEditor)),
+            StorageRoles.legacyBucketReader(),
+            new HashSet<>(Collections.singleton(projectViewer)),
+            StorageRoles.legacyObjectReader(),
+            (Set<Identity>) new HashSet<>(Collections.singleton(Identity.allUsers())));
+
+    // Validate getting policy.
+    Policy currentPolicy = storage.getIamPolicy(BUCKET_REQUESTER_PAYS, bucketOptions);
+    assertEquals(bindingsWithoutPublicRead, currentPolicy.getBindings());
+
+    // Validate updating policy.
+    Policy updatedPolicy =
+        storage.setIamPolicy(
+            BUCKET_REQUESTER_PAYS,
+            currentPolicy
+                .toBuilder()
+                .addIdentity(StorageRoles.legacyObjectReader(), Identity.allUsers())
+                .build(),
+            bucketOptions);
+    assertEquals(bindingsWithPublicRead, updatedPolicy.getBindings());
+    Policy revertedPolicy =
+        storage.setIamPolicy(
+            BUCKET_REQUESTER_PAYS,
+            updatedPolicy
+                .toBuilder()
+                .removeIdentity(StorageRoles.legacyObjectReader(), Identity.allUsers())
+                .build(),
+            bucketOptions);
+    assertEquals(bindingsWithoutPublicRead, revertedPolicy.getBindings());
+
+    // Validate testing permissions.
+    List<Boolean> expectedPermissions = ImmutableList.of(true, true);
+    assertEquals(
+        expectedPermissions,
+        storage.testIamPermissions(
+            BUCKET_REQUESTER_PAYS,
+            ImmutableList.of("storage.buckets.getIamPolicy", "storage.buckets.setIamPolicy"),
+            bucketOptions));
+    Bucket bucketFalse =
+        storage.update(
+            bucketTrue.toBuilder().setRequesterPays(false).build(),
+            Storage.BucketTargetOption.userProject(projectId));
+    assertFalse(bucketFalse.requesterPays());
   }
 
   @Test
@@ -2763,9 +2774,12 @@ public class ITStorageTest {
 
   @Test
   public void testUpdateBucketRequesterPays() {
+    unsetRequesterPays();
     Bucket remoteBucket =
-        storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
-    assertFalse(remoteBucket.requesterPays());
+        storage.get(
+            BUCKET_REQUESTER_PAYS,
+            Storage.BucketGetOption.fields(BucketField.ID, BucketField.BILLING));
+    assertTrue(remoteBucket.requesterPays() == null || !remoteBucket.requesterPays());
     remoteBucket = remoteBucket.toBuilder().setRequesterPays(true).build();
     Bucket updatedBucket = storage.update(remoteBucket);
     assertTrue(updatedBucket.requesterPays());
@@ -2776,7 +2790,8 @@ public class ITStorageTest {
     Blob remoteBlob = updatedBucket.create(blobName, BLOB_BYTE_CONTENT, option);
     assertNotNull(remoteBlob);
     byte[] readBytes =
-        storage.readAllBytes(BUCKET, blobName, Storage.BlobSourceOption.userProject(projectId));
+        storage.readAllBytes(
+            BUCKET_REQUESTER_PAYS, blobName, Storage.BlobSourceOption.userProject(projectId));
     assertArrayEquals(BLOB_BYTE_CONTENT, readBytes);
     remoteBucket = remoteBucket.toBuilder().setRequesterPays(false).build();
     updatedBucket = storage.update(remoteBucket, Storage.BucketTargetOption.userProject(projectId));
@@ -3022,7 +3037,7 @@ public class ITStorageTest {
     } catch (StorageException ex) {
       // expected
     } finally {
-      remoteBlob.toBuilder().setEventBasedHold(false).build().update();
+      remoteBlob.toBuilder().setTemporaryHold(false).build().update();
     }
   }
 
@@ -3474,4 +3489,50 @@ public class ITStorageTest {
   public void testAutoContentTypeWriter() throws IOException {
     testAutoContentType("writer");
   }
+
+  @Test
+  public void testRemoveBucketCORS() throws ExecutionException, InterruptedException {
+    String bucketName = RemoteStorageHelper.generateBucketName();
+    List<Cors.Origin> origins = ImmutableList.of(Cors.Origin.of("http://cloud.google.com"));
+    List<HttpMethod> httpMethods = ImmutableList.of(HttpMethod.GET);
+    List<String> responseHeaders = ImmutableList.of("Content-Type");
+    try {
+      Cors cors =
+          Cors.newBuilder()
+              .setOrigins(origins)
+              .setMethods(httpMethods)
+              .setResponseHeaders(responseHeaders)
+              .setMaxAgeSeconds(100)
+              .build();
+      storage.create(BucketInfo.newBuilder(bucketName).setCors(ImmutableList.of(cors)).build());
+
+      // case-1 : Cors are set and field selector is selected then returns not-null.
+      Bucket remoteBucket =
+          storage.get(bucketName, Storage.BucketGetOption.fields(BucketField.CORS));
+      assertThat(remoteBucket.getCors()).isNotNull();
+      assertThat(remoteBucket.getCors().get(0).getMaxAgeSeconds()).isEqualTo(100);
+      assertThat(remoteBucket.getCors().get(0).getMethods()).isEqualTo(httpMethods);
+      assertThat(remoteBucket.getCors().get(0).getOrigins()).isEqualTo(origins);
+      assertThat(remoteBucket.getCors().get(0).getResponseHeaders()).isEqualTo(responseHeaders);
+
+      // case-2 : Cors are set but field selector isn't selected then returns not-null.
+      remoteBucket = storage.get(bucketName);
+      assertThat(remoteBucket.getCors()).isNotNull();
+
+      // Remove CORS configuration from the bucket.
+      Bucket updatedBucket = remoteBucket.toBuilder().setCors(null).build().update();
+      assertThat(updatedBucket.getCors()).isNull();
+
+      // case-3 : Cors are not set and field selector is selected then returns null.
+      updatedBucket = storage.get(bucketName, Storage.BucketGetOption.fields(BucketField.CORS));
+      assertThat(updatedBucket.getCors()).isNull();
+
+      // case-4 : Cors are not set and field selector isn't selected then returns null.
+      updatedBucket = storage.get(bucketName);
+      assertThat(updatedBucket.getCors()).isNull();
+
+    } finally {
+      RemoteStorageHelper.forceDelete(storage, bucketName, 5, TimeUnit.SECONDS);
+    }
+  }
 }