pvary commented on code in PR #13831: URL: https://github.com/apache/iceberg/pull/13831#discussion_r2282584449
########## flink/v2.0/flink/src/test/java/org/apache/iceberg/flink/maintenance/operator/TestDeleteFilesProcessor.java: ########## @@ -74,27 +79,172 @@ void testDeleteMissingFile() throws Exception { Path dummyFile = FileSystems.getDefault().getPath(table.location().substring(5), DUMMY_FILE_NAME); - deleteFile(tableLoader(), dummyFile.toString()); + deleteFile(tableLoader(), dummyFile.toString(), true); assertThat(listFiles(table)).isEqualTo(TABLE_FILES); } @Test void testInvalidURIScheme() throws Exception { - deleteFile(tableLoader(), "wrong://"); + deleteFile(tableLoader(), "wrong://", false); assertThat(listFiles(table)).isEqualTo(TABLE_FILES); } - private void deleteFile(TableLoader tableLoader, String fileName) throws Exception { - tableLoader().open(); + @Test + void testDeleteNonExistentFile() throws Exception { + String nonexistentFile = "nonexistentFile.txt"; + + deleteFile(tableLoader(), nonexistentFile, true); + + assertThat(listFiles(table)).isEqualTo(TABLE_FILES); + } + + @Test + void testDeleteLargeFile() throws Exception { + // Simulate a large file (e.g., 100MB file) + String largeFileName = "largeFile.txt"; + Path largeFile = Path.of(tablePath(table).toString(), largeFileName); + + // Write a large file to disk (this will simulate the large file in the filesystem) + byte[] largeData = new byte[1024 * 1024 * 100]; // 100 MB + Files.write(largeFile, largeData); + + // Verify that the file was created + Set<String> files = listFiles(table); + assertThat(files).contains(largeFileName); + + // Use the DeleteFilesProcessor to delete the large file + deleteFile(tableLoader(), largeFile.toString(), true); + + // Verify that the large file has been deleted + files = listFiles(table); + assertThat(files).doesNotContain(largeFileName); + } + + private void deleteFile(TableLoader tableLoader, String fileName, boolean expectSuccess) Review Comment: We usually try to avoid having boolean parameters, because it is hard to read the code without visiting the method declaration and explicitly checking the parameter. We sometimes accept this in test code, but please leave a comment where they are called. Something like this: ``` deleteFile(tableLoader(), nonexistentFile, true /* expectSuccess */); ``` -- 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. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org