From common-issues-return-207959-apmail-hadoop-common-issues-archive=hadoop.apache.org@hadoop.apache.org Tue Oct 6 12:59:57 2020 Return-Path: X-Original-To: apmail-hadoop-common-issues-archive@locus.apache.org Delivered-To: apmail-hadoop-common-issues-archive@locus.apache.org Received: from mailroute1-lw-us.apache.org (mailroute1-lw-us.apache.org [207.244.88.153]) by minotaur.apache.org (Postfix) with ESMTP id AA5951A0C2 for ; Tue, 6 Oct 2020 12:59:57 +0000 (UTC) Received: from mail.apache.org (localhost [127.0.0.1]) by mailroute1-lw-us.apache.org (ASF Mail Server at mailroute1-lw-us.apache.org) with SMTP id 5B70012264B for ; Tue, 6 Oct 2020 12:59:57 +0000 (UTC) Received: (qmail 13070 invoked by uid 500); 6 Oct 2020 12:59:56 -0000 Delivered-To: apmail-hadoop-common-issues-archive@hadoop.apache.org Received: (qmail 13038 invoked by uid 500); 6 Oct 2020 12:59:56 -0000 Mailing-List: contact common-issues-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list common-issues@hadoop.apache.org Received: (qmail 13022 invoked by uid 99); 6 Oct 2020 12:59:56 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 06 Oct 2020 12:59:56 +0000 From: =?utf-8?q?GitBox?= To: common-issues@hadoop.apache.org Subject: =?utf-8?q?=5BGitHub=5D_=5Bhadoop=5D_steveloughran_commented_on_a_change_in_p?= =?utf-8?q?ull_request_=232354=3A_HADOOP-17281_Implement_FileSystem=2ElistSt?= =?utf-8?q?atusIterator=28=29_in_S3AFileSystem?= Message-ID: <160198919658.32230.12259046092846850701.asfpy@gitbox.apache.org> Date: Tue, 06 Oct 2020 12:59:56 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit In-Reply-To: References: 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 statusList = (List) iteratorToList( getFileSystem().listStatusIterator(f)); - assertEquals("size of file list returned", 1, statusList.size()); - assertIsNamedFile(f, statusList.get(0)); + validateListingForFile(f, statusList, false); + List statusList2 = (List) 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 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