steveloughran commented on a change in pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#discussion_r500254718



##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java
##########
@@ -1520,7 +1521,8 @@ public static TreeScanResults treeWalk(FileSystem fs, Path path)
       while (true) {
         list.add(iterator.next());
       }
-    } catch (NoSuchElementException expected) {
+    } catch (NoSuchElementException | IllegalStateException expected) {

Review comment:
       as discussed: better to fix DirListingIterator to match RemoteIterator API; we can consider it's failure a bug

##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractGetFileStatusTest.java
##########
@@ -359,16 +357,39 @@ public void testListStatusFile() throws Throwable {
   public void testListStatusIteratorFile() throws Throwable {
     describe("test the listStatusIterator(path) on a file");
     Path f = touchf("listStItrFile");
+
     List<FileStatus> statusList = (List<FileStatus>) iteratorToList(
             getFileSystem().listStatusIterator(f));
-    assertEquals("size of file list returned", 1, statusList.size());
-    assertIsNamedFile(f, statusList.get(0));
+    validateListingForFile(f, statusList, false);
+
     List<FileStatus> statusList2 =
             (List<FileStatus>) iteratorToListThroughNextCallsAlone(
                     getFileSystem().listStatusIterator(f));
-    assertEquals("size of file list returned through next() calls",
-            1, statusList2.size());
-    assertIsNamedFile(f, statusList2.get(0));
+    validateListingForFile(f, statusList2, true);
+  }
+
+  /**
+   * Validate listing result for an input path which is file.
+   * @param f file.
+   * @param statusList list status of a file.
+   * @param nextCallAlone whether the listing generated just using
+   *                      next() calls.
+   */
+  private void validateListingForFile(Path f,
+                                      List<FileStatus> statusList,
+                                      boolean nextCallAlone) {
+    String msg = String.format("size of file list returned using %s should " +
+            "be 1", nextCallAlone ?
+            "next() calls alone" : "hasNext() and next() calls");
+    Assertions.assertThat(statusList)
+            .describedAs(msg)
+            .hasSize(1);
+    Assertions.assertThat(statusList.get(0).getPath().toString())
+            .describedAs("path returned should match with the input path")

Review comment:
       any reason not to leave both as Path and let Path.equals() to do the work? It compares URIs




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org