reviewer and linter comments

jean-philippe-martin · jean-philippe-martin · commit 0757985e13f8 · 2017-03-06T13:13:14.000-08:00
diff --git a/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java b/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java
@@ -28,7 +28,6 @@
 import static java.nio.file.StandardOpenOption.WRITE;
 
 import com.google.common.collect.ImmutableList;
-import com.google.common.net.UrlEscapers;
 import com.google.common.testing.NullPointerTester;
 
 import org.junit.Before;
@@ -655,28 +654,26 @@ public void testNewFileSystem() throws IOException {
   }
 
   @Test
-  public void testFromSpace() throws UnsupportedEncodingException {
+  public void testFromSpace() throws UnsupportedEncodingException, IOException {
     // User should be able to create paths to files whose name contains a space.
     // Traditional way 1: manually escape the spaces
     Path path1 = Paths.get(URI.create("gs://bucket/with/a%20space"));
     CloudStorageFileSystemProvider provider =
         (CloudStorageFileSystemProvider)path1.getFileSystem().provider();
-    // Traditional way 2: use a library to escape the spaces
-    String escaped = UrlEscapers.urlFragmentEscaper().escape("gs://bucket/with/a space");
-    Path path2 = Paths.get(URI.create(escaped));
+    // Traditional way 2: use UrlEscapers.urlFragmentEscaper().escape
+    // to escape the string for you.
+    // (Not tested because UrlEscapers isn't the unit under test).
+
     // Non-traditional way: use our convenience method to work around URIs not being allowed to
     // contain spaces.
     Path path3 = provider.getPath("gs://bucket/with/a space");
-    // All 3 should be equivalent
-    assertThat(path1.getFileSystem().provider()).isEqualTo(path2.getFileSystem().provider());
-    assertThat(path2.getFileSystem().provider()).isEqualTo(path3.getFileSystem().provider());
-    assertThat(path1.toUri()).isEqualTo(path2.toUri());
-    assertThat(path2.toUri()).isEqualTo(path3.toUri());
+    // Both approaches should be equivalent
+    assertThat(path1.getFileSystem().provider()).isEqualTo(path3.getFileSystem().provider());
+    assertThat(path1.toUri()).isEqualTo(path3.toUri());
 
+    // getPath does not interpret the string at all.
     Path path4 = provider.getPath("gs://bucket/with/a%20percent");
-    String toString = path4.toString();
-    assertThat(toString).doesNotContain(" ");
-    assertThat(toString).contains("%");
+    assertThat(path4.toString()).isEqualTo("/with/a%20percent");
   }
 
   private static CloudStorageConfiguration permitEmptyPathComponents(boolean value) {
diff --git a/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStoragePathTest.java b/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStoragePathTest.java
@@ -489,16 +489,13 @@ public void testNullness() throws IOException, NoSuchMethodException, SecurityEx
   public void testSpaces() throws IOException {
     try (CloudStorageFileSystem fs = CloudStorageFileSystem.forBucket("doodle")) {
       Path path = fs.getPath("/with/a space");
-      String toString = path.toString();
-      assertThat(toString).contains(" ");
       // we can also go via a URI. Decoding should give us the space back.
       String toUri = URLDecoder.decode(path.toUri().toString(), "UTF-8");
-      assertThat(toUri).contains(" ");
+      assertThat(toUri).isEqualTo("gs://doodle/with/a space");
 
       Path path2 = fs.getPath("/with/a%20percent");
       String toUri2 = URLDecoder.decode(path2.toUri().toString(), "UTF-8");
-      assertThat(toUri2).doesNotContain(" ");
-      assertThat(toUri2).contains("%");
+      assertThat(toUri2).isEqualTo("gs://doodle/with/a%20percent");
     }
   }
 
