nastra commented on code in PR #12568:
URL: https://github.com/apache/iceberg/pull/12568#discussion_r2002553616


##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteTablePathsAction.java:
##########
@@ -932,23 +1093,40 @@ protected void checkFileNum(
       int manifestFileCount,
       int totalCount,
       RewriteTablePath.Result result) {
+    checkFileNum(versionFileCount, manifestListCount, manifestFileCount, 0, 
totalCount, result);
+  }
+
+  protected void checkFileNum(
+      int versionFileCount,
+      int manifestListCount,
+      int manifestFileCount,
+      int statisticsFileCount,
+      int totalCount,
+      RewriteTablePath.Result result) {
     List<String> filesToMove =
         spark
             .read()
             .format("text")
             .load(result.fileListLocation())
             .as(Encoders.STRING())
             .collectAsList();
-    assertThat(filesToMove.stream().filter(f -> 
f.endsWith(".metadata.json")).count())
-        .withFailMessage("Wrong rebuilt version file count")
+    Predicate<String> isManifest = f -> f.endsWith("-m0.avro") || 
f.endsWith("-m1.avro");
+    Predicate<String> isManifestList = f -> f.contains("snap-") && 
f.endsWith(".avro");
+    Predicate<String> isMetadataJSON = f -> f.endsWith(".metadata.json");
+
+    assertThat(filesToMove.stream().filter(isMetadataJSON).count())
+        .as("Wrong rebuilt version file count")
         .isEqualTo(versionFileCount);
-    assertThat(filesToMove.stream().filter(f -> f.contains("snap-")).count())
-        .withFailMessage("Wrong rebuilt Manifest list file count")
+    assertThat(filesToMove.stream().filter(isManifestList).count())
+        .as("Wrong rebuilt Manifest list file count")
         .isEqualTo(manifestListCount);
-    assertThat(filesToMove.stream().filter(f -> 
f.endsWith("-m0.avro")).count())
-        .withFailMessage("Wrong rebuilt Manifest file file count")
+    assertThat(filesToMove.stream().filter(isManifest).count())
+        .as("Wrong rebuilt Manifest file file count")
         .isEqualTo(manifestFileCount);
-    assertThat(filesToMove.size()).withFailMessage("Wrong total file 
count").isEqualTo(totalCount);
+    assertThat(filesToMove.stream().filter(f -> f.endsWith(".stats")).count())
+        .withFailMessage("Wrong rebuilt Statistic file count")

Review Comment:
   can you please update all of the places in this test class that use 
`withFailMessage()` to `as()`? Using `withFailMessage()` is typically not 
what's desired as it will only show the message but not the values of 
expected/actual, thus making it more difficult to debug when tests fail



-- 
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

Reply via email to