[GitHub] flink pull request #6263: [FLINK-9743][Client] Use correct zip/jar path sepa...
Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/6263 ---
[GitHub] flink pull request #6263: [FLINK-9743][Client] Use correct zip/jar path sepa...
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/6263#discussion_r201322209
--- Diff:
flink-clients/src/test/java/org/apache/flink/client/program/PackagedProgramTest.java
---
@@ -21,16 +21,24 @@
import org.apache.flink.client.cli.CliFrontendTestUtils;
import org.junit.Assert;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
import java.io.File;
+import java.io.FileOutputStream;
import java.io.PrintStream;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
/**
* Tests for the {@link PackagedProgramTest}.
*/
public class PackagedProgramTest {
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
--- End diff --
can be final
---
[GitHub] flink pull request #6263: [FLINK-9743][Client] Use correct zip/jar path sepa...
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/6263#discussion_r201291273
--- Diff:
flink-clients/src/test/java/org/apache/flink/client/program/PackagedProgramTest.java
---
@@ -56,6 +63,54 @@ public void testGetPreviewPlan() {
}
}
+ /**
+* The test for {@link PackagedProgram#extractContainedLibraries}.
+* As a prerequisite the test generates a jar file with the following
structure
+* test.jar
+* |- lib
+* |--|- internalTest.jar
+*/
+ @Test
+ public void testExtractContainedLibraries() {
+ String s = "testExtractContainedLibraries";
+ Path workDir = null;
--- End diff --
use junit `TemporaryFolder` instead
---
[GitHub] flink pull request #6263: [FLINK-9743][Client] Use correct zip/jar path sepa...
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/6263#discussion_r201291489
--- Diff:
flink-clients/src/test/java/org/apache/flink/client/program/PackagedProgramTest.java
---
@@ -56,6 +63,54 @@ public void testGetPreviewPlan() {
}
}
+ /**
+* The test for {@link PackagedProgram#extractContainedLibraries}.
+* As a prerequisite the test generates a jar file with the following
structure
+* test.jar
+* |- lib
+* |--|- internalTest.jar
+*/
+ @Test
+ public void testExtractContainedLibraries() {
+ String s = "testExtractContainedLibraries";
+ Path workDir = null;
+ Path fakeJar = null;
+ try {
+ workDir =
Files.createTempDirectory(PackagedProgram.class.getSimpleName() + "_");
+ fakeJar =
Paths.get(workDir.toAbsolutePath().toString(), "test.jar");
+ FileOutputStream fos = new
FileOutputStream(fakeJar.toFile());
+ try (ZipOutputStream zos = new ZipOutputStream(fos)) {
+ ZipEntry entry = new
ZipEntry("lib/internalTest.jar");
+ zos.putNextEntry(entry);
+ zos.write(s.getBytes());
+ zos.closeEntry();
+ } catch (IOException e) {
+ System.err.println(e.getMessage());
+ e.printStackTrace();
+ Assert.fail("Test is erroneous: " +
e.getMessage());
+ }
+
PackagedProgram.extractContainedLibraries(fakeJar.toUri().toURL());
+ }
+ catch (Exception e) {
+ System.err.println(e.getMessage());
+ e.printStackTrace();
+ Assert.fail("Test is erroneous: " + e.getMessage());
+ } finally {
--- End diff --
this block will be unnecessary with the usage of `TemporaryFolder`
---
[GitHub] flink pull request #6263: [FLINK-9743][Client] Use correct zip/jar path sepa...
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/6263#discussion_r201291422
--- Diff:
flink-clients/src/test/java/org/apache/flink/client/program/PackagedProgramTest.java
---
@@ -56,6 +63,54 @@ public void testGetPreviewPlan() {
}
}
+ /**
+* The test for {@link PackagedProgram#extractContainedLibraries}.
+* As a prerequisite the test generates a jar file with the following
structure
+* test.jar
+* |- lib
+* |--|- internalTest.jar
+*/
+ @Test
+ public void testExtractContainedLibraries() {
+ String s = "testExtractContainedLibraries";
+ Path workDir = null;
+ Path fakeJar = null;
+ try {
+ workDir =
Files.createTempDirectory(PackagedProgram.class.getSimpleName() + "_");
+ fakeJar =
Paths.get(workDir.toAbsolutePath().toString(), "test.jar");
+ FileOutputStream fos = new
FileOutputStream(fakeJar.toFile());
+ try (ZipOutputStream zos = new ZipOutputStream(fos)) {
+ ZipEntry entry = new
ZipEntry("lib/internalTest.jar");
+ zos.putNextEntry(entry);
+ zos.write(s.getBytes());
+ zos.closeEntry();
+ } catch (IOException e) {
+ System.err.println(e.getMessage());
+ e.printStackTrace();
+ Assert.fail("Test is erroneous: " +
e.getMessage());
+ }
+
PackagedProgram.extractContainedLibraries(fakeJar.toUri().toURL());
+ }
+ catch (Exception e) {
--- End diff --
no need to catch them, just let them bubble up
---
[GitHub] flink pull request #6263: [FLINK-9743][Client] Use correct zip/jar path sepa...
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/6263#discussion_r201291547
--- Diff:
flink-clients/src/test/java/org/apache/flink/client/program/PackagedProgramTest.java
---
@@ -56,6 +63,54 @@ public void testGetPreviewPlan() {
}
}
+ /**
+* The test for {@link PackagedProgram#extractContainedLibraries}.
+* As a prerequisite the test generates a jar file with the following
structure
+* test.jar
+* |- lib
+* |--|- internalTest.jar
+*/
+ @Test
+ public void testExtractContainedLibraries() {
+ String s = "testExtractContainedLibraries";
+ Path workDir = null;
+ Path fakeJar = null;
+ try {
+ workDir =
Files.createTempDirectory(PackagedProgram.class.getSimpleName() + "_");
+ fakeJar =
Paths.get(workDir.toAbsolutePath().toString(), "test.jar");
+ FileOutputStream fos = new
FileOutputStream(fakeJar.toFile());
+ try (ZipOutputStream zos = new ZipOutputStream(fos)) {
+ ZipEntry entry = new
ZipEntry("lib/internalTest.jar");
+ zos.putNextEntry(entry);
+ zos.write(s.getBytes());
+ zos.closeEntry();
+ } catch (IOException e) {
--- End diff --
again, just let hte exception bubble up.
---
[GitHub] flink pull request #6263: [FLINK-9743][Client] Use correct zip/jar path sepa...
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/6263#discussion_r201291704
--- Diff:
flink-clients/src/test/java/org/apache/flink/client/program/PackagedProgramTest.java
---
@@ -56,6 +63,54 @@ public void testGetPreviewPlan() {
}
}
+ /**
+* The test for {@link PackagedProgram#extractContainedLibraries}.
+* As a prerequisite the test generates a jar file with the following
structure
+* test.jar
+* |- lib
+* |--|- internalTest.jar
+*/
+ @Test
+ public void testExtractContainedLibraries() {
+ String s = "testExtractContainedLibraries";
+ Path workDir = null;
+ Path fakeJar = null;
+ try {
+ workDir =
Files.createTempDirectory(PackagedProgram.class.getSimpleName() + "_");
+ fakeJar =
Paths.get(workDir.toAbsolutePath().toString(), "test.jar");
+ FileOutputStream fos = new
FileOutputStream(fakeJar.toFile());
+ try (ZipOutputStream zos = new ZipOutputStream(fos)) {
--- End diff --
inline `FileOutputStream` creation, i.e. `new ZipOutputStream(new
FileOutputStream(...))`
---
[GitHub] flink pull request #6263: [FLINK-9743][Client] Use correct zip/jar path sepa...
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/6263#discussion_r201291316
--- Diff:
flink-clients/src/test/java/org/apache/flink/client/program/PackagedProgramTest.java
---
@@ -56,6 +63,54 @@ public void testGetPreviewPlan() {
}
}
+ /**
+* The test for {@link PackagedProgram#extractContainedLibraries}.
+* As a prerequisite the test generates a jar file with the following
structure
+* test.jar
+* |- lib
+* |--|- internalTest.jar
+*/
+ @Test
+ public void testExtractContainedLibraries() {
+ String s = "testExtractContainedLibraries";
+ Path workDir = null;
+ Path fakeJar = null;
+ try {
+ workDir =
Files.createTempDirectory(PackagedProgram.class.getSimpleName() + "_");
--- End diff --
derive from `TemporaryFolder`
---
[GitHub] flink pull request #6263: [FLINK-9743][Client] Use correct zip/jar path sepa...
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6263 [FLINK-9743][Client] Use correct zip/jar path separator ## What is the purpose of the change *This PR resolves libraries extraction issue from jars* ## Brief change log - *Always in case of zip/jar use '/' path separator* - *Test with generated jar emulating the real case* ## Verifying this change - Added test generates fake jar with a structure test.jar |- lib |--|- internalTest.jar and then calls for `PackagedProgram#extractContainedLibraries` to check if it extracts internalTest.jar correctly ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not applicable) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink FLINK_9743 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6263.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6263 commit 649b9f1a0893aea32bd2dc3cfa1702bfb77ab29f Author: snuyanzin Date: 2018-07-05T08:58:33Z [FLINK-9743] use correct zip path separator, PackagedProgramTest#testExtractContainedLibraries to check PackagedProgram#extractContainedLibraries ---
