review: Added test + Improved ArchiveAssertions

Keep tarEntry computation+configuration logic in the same method

Signed-off-by: Marc Nuri <marc@marcnuri.com>

jkube-kit/common-test/src/main/java/org/eclipse/jkube/kit/common/assertj/ArchiveAssertions.java

@@ -19,6 +19,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
 import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
@@ -92,20 +93,29 @@ private TarArchiveInputStream inputStream(BufferedInputStream bis) {
     }
   }
 
-  public AbstractListAssert<ListAssert<String>, List<? extends String>, String, ObjectAssert<String>> fileTree()
-      throws IOException {
-    final List<String> archiveFileTree = new ArrayList<>();
-
+  private List<TarArchiveEntry> loadEntries() throws IOException {
+    final List<TarArchiveEntry> archiveEntries = new ArrayList<>();
     try (
         FileInputStream fis = new FileInputStream(actual);
         BufferedInputStream bis = new BufferedInputStream(fis);
         TarArchiveInputStream tis = inputStream(bis)
     ) {
       TarArchiveEntry entry;
       while ((entry = tis.getNextTarEntry()) != null) {
-        archiveFileTree.add(entry.getName());
+        archiveEntries.add(entry);
       }
     }
-    return org.assertj.core.api.Assertions.assertThat(archiveFileTree);
+    return archiveEntries;
+  }
+
+  public AbstractListAssert<ListAssert<TarArchiveEntry>, List<? extends TarArchiveEntry>, TarArchiveEntry, ObjectAssert<TarArchiveEntry>> entries()
+      throws IOException {
+    return org.assertj.core.api.Assertions.assertThat(loadEntries());
+  }
+
+  public AbstractListAssert<ListAssert<String>, List<? extends String>, String, ObjectAssert<String>> fileTree()
+      throws IOException {
+    return org.assertj.core.api.Assertions.assertThat(
+        loadEntries().stream().map(TarArchiveEntry::getName).collect(Collectors.toList()));
   }
 }

jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/archive/JKubeTarArchiver.java

@@ -74,7 +74,14 @@ public static File createTarBall(
         String relativeFilePath = inputDirectory.toURI().relativize(
             new File(currentFile.getAbsolutePath()).toURI()).getPath();
 
-        final TarArchiveEntry tarEntry = createTarArchiveEntry(fileModeMap, currentFile, relativeFilePath);
+        final TarArchiveEntry tarEntry = new TarArchiveEntry(currentFile, relativeFilePath);
+        tarEntry.setSize(currentFile.length());
+        if (fileModeMap.containsKey(currentFile)) {
+          tarEntry.setMode(Integer.parseInt(fileModeMap.get(currentFile), 8));
+        } else if (currentFile.isDirectory()) {
+          tarEntry.setSize(0L);
+          tarEntry.setMode(TarArchiveEntry.DEFAULT_DIR_MODE);
+        }
         Optional.ofNullable(tarArchiveEntryCustomizer).ifPresent(tac -> tac.accept(tarEntry));
         tarArchiveOutputStream.putArchiveEntry(tarEntry);
         if (currentFile.isFile()) {
@@ -87,16 +94,4 @@ public static File createTarBall(
 
     return outputFile;
   }
-
-  static TarArchiveEntry createTarArchiveEntry(Map<File, String> fileModeMap, File currentFile, String relativeFilePath) {
-    final TarArchiveEntry tarEntry = new TarArchiveEntry(currentFile, relativeFilePath);
-    tarEntry.setSize(currentFile.length());
-    if (fileModeMap.containsKey(currentFile)) {
-      tarEntry.setMode(Integer.parseInt(fileModeMap.get(currentFile), 8));
-    } else if (currentFile.isDirectory()) {
-      tarEntry.setSize(0L);
-      tarEntry.setMode(TarArchiveEntry.DEFAULT_DIR_MODE);
-    }
-    return tarEntry;
-  }
 }

jkube-kit/common/src/test/java/org/eclipse/jkube/kit/common/archive/JKubeTarArchiverTest.java

@@ -17,7 +17,6 @@
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.function.Consumer;
 
 import org.eclipse.jkube.kit.common.assertj.ArchiveAssertions;
@@ -31,7 +30,7 @@
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
-import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.Assertions.tuple;
 
 public class JKubeTarArchiverTest {
 
@@ -156,31 +155,44 @@ public void createTarBallOfDirectory_defaultCompressionWithTarCustomizer_creates
   }
 
   @Test
-  public void createTarArchiveEntry_whenUsedForDirectories_shouldCreateEntryWithZeroSize() {
+  public void createTarBallOfDirectory_defaultCompression_createsTarWithCorrectEntrySizeAndModeForDirectories() throws Exception {
     // Given
-    File tmpDir = new File(getClass().getResource("/jkubetararchiver-directory/").getFile());
-
+    final int defaultDirMode = Integer.parseInt("040755", 8); // 16877
+    final File outputFile = temporaryFolder.newFile("target.noExtension");
     // When
-    TarArchiveEntry tarArchiveEntry = JKubeTarArchiver.createTarArchiveEntry(Collections.emptyMap(), tmpDir, "/tmp/foo");
-
+    final File result = JKubeTarArchiver.createTarBallOfDirectory(outputFile, toCompress, ArchiveCompression.none);
     // Then
-    assertThat(tarArchiveEntry)
-            .isNotNull()
-            .hasFieldOrPropertyWithValue("size", 0L);
+    ArchiveAssertions.assertThat(result)
+        .isSameAs(outputFile)
+        .isNotEmpty()
+        .entries()
+        .filteredOn(TarArchiveEntry::isDirectory)
+        .hasSize(2)
+        .extracting("name", "size", "mode")
+        .containsExactlyInAnyOrder(
+            tuple("nested/", 0L, defaultDirMode),
+            tuple("nested/directory/", 0L, defaultDirMode)
+        );
   }
 
   @Test
-  public void createTarArchiveEntry_whenUsedForFiles_shouldCreateEntryWithNonZeroSize() {
+  public void createTarBallOfDirectory_defaultCompression_createsTarWithCorrectEntrySizeAndModeForFiles() throws Exception {
     // Given
-    File tmpFile = new File(getClass().getResource("/jkubetararchiver-directory/jkubetararchiver-non-empty-file.txt").getFile());
-
+    final int defaultFileMode = Integer.parseInt("0100644", 8); // 33188
+    final File outputFile = temporaryFolder.newFile("target.noExtension");
     // When
-    TarArchiveEntry tarArchiveEntry = JKubeTarArchiver.createTarArchiveEntry(Collections.singletonMap(tmpFile, "0644"), tmpFile, "/tmp/foo");
-
+    final File result = JKubeTarArchiver.createTarBallOfDirectory(outputFile, toCompress, ArchiveCompression.none);
     // Then
-    assertThat(tarArchiveEntry)
-            .isNotNull()
-            .hasFieldOrPropertyWithValue("size", 10L)
-            .hasFieldOrPropertyWithValue("mode", Integer.parseInt("0644", 8));
+    ArchiveAssertions.assertThat(result)
+        .isSameAs(outputFile)
+        .isNotEmpty()
+        .entries()
+        .filteredOn(TarArchiveEntry::isFile)
+        .hasSize(2)
+        .extracting("name", "size", "mode")
+        .containsExactlyInAnyOrder(
+            tuple("file.txt", 12L, defaultFileMode),
+            tuple("nested/directory/" + LONG_FILE_NAME, 19L, defaultFileMode)
+        );
   }
 }