You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by srowen <gi...@git.apache.org> on 2018/06/02 18:40:31 UTC

[GitHub] spark pull request #21456: [SPARK-24356] [CORE] Duplicate strings in File.pa...

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21456#discussion_r192567034
  
    --- Diff: common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolverSuite.java ---
    @@ -135,4 +136,23 @@ public void jsonSerializationOfExecutorRegistration() throws IOException {
           "\"subDirsPerLocalDir\": 7, \"shuffleManager\": " + "\"" + SORT_MANAGER + "\"}";
         assertEquals(shuffleInfo, mapper.readValue(legacyShuffleJson, ExecutorShuffleInfo.class));
       }
    +
    +  @Test
    +  public void testNormalizeAndInternPathname() {
    +    assertPathsMatch("/foo", "bar", "baz", "/foo/bar/baz");
    +    assertPathsMatch("//foo/", "bar/", "//baz", "/foo/bar/baz");
    +    assertPathsMatch("foo", "bar", "baz///", "foo/bar/baz");
    +    assertPathsMatch("/foo/", "/bar//", "/baz", "/foo/bar/baz");
    +    assertPathsMatch("/", "", "", "/");
    +    assertPathsMatch("/", "/", "/", "/");
    +  }
    +
    +  private void assertPathsMatch(String p1, String p2, String p3, String expectedPathname) {
    +    String normPathname =
    +        ExternalShuffleBlockResolver.createNormalizedInternedPathname(p1, p2, p3);
    +    assertEquals(expectedPathname, normPathname);
    +    File file = new File(normPathname);
    +    String returnedPath = file.getPath();
    +    assertTrue(normPathname == returnedPath);
    --- End diff --
    
    Last nit, should this be `assertSame` if you're asserting they're the same string object exactly?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org