attilapiros commented on a change in pull request #28967:
URL: https://github.com/apache/spark/pull/28967#discussion_r450696105
##########
File path:
common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleBlockResolverSuite.java
##########
@@ -145,29 +143,4 @@ public void jsonSerializationOfExecutorRegistration()
throws IOException {
assertEquals(shuffleInfo, mapper.readValue(legacyShuffleJson,
ExecutorShuffleInfo.class));
}
- @Test
- public void testNormalizeAndInternPathname() {
Review comment:
@dongjoon-hyun sure, here you are:
The `createNormalizedInternedPathname` was as good as it was close to
`java.io.FileSystem#normalize()` as in the old code the String interning could
only save as any memory when its result was in equal with
`java.io.FileSystem#normalize()` which was called within `File` constructor. If
there was any difference in the string then the path in `File` would use a
different (not interned) string as that string would be transformed a bit more.
When the test was created in the first place if they could call
`java.io.FileSystem#normalize()` somehow within the tests only then they would
given asserts where the expected result of `createNormalizedInternedPathname`
would be `java.io.FileSystem#normalize()` (instead of the hardcoded string
paths). The test should check whether the same transformation is done on the
incoming path depending on the OS.
So now as we can call indirectly the `java.io.FileSystem#normalize()` we
could rewrite the old test but that would mean having assert where
`java.io.FileSystem#normalize()` is checked whether it is really
`java.io.FileSystem#normalize()`. This would be a trivial assert as it is
always true (like an `assertTrue(true)` just longer). So this is why we do not
need the old tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
