wangyang0918 commented on a change in pull request #15020: URL: https://github.com/apache/flink/pull/15020#discussion_r589957082
########## File path: flink-clients/src/main/java/org/apache/flink/client/deployment/application/AbstractPackagedProgramRetriever.java ########## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.client.deployment.application; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.client.program.PackagedProgram; +import org.apache.flink.client.program.PackagedProgramRetriever; +import org.apache.flink.client.program.ProgramInvocationException; +import org.apache.flink.configuration.Configuration; +import org.apache.flink.util.FileUtils; +import org.apache.flink.util.FlinkException; +import org.apache.flink.util.function.FunctionUtils; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import java.io.File; +import java.io.IOException; +import java.net.URL; +import java.nio.file.Path; +import java.util.Collection; +import java.util.Collections; +import java.util.stream.Collectors; + +import static java.util.Objects.requireNonNull; + +/** + * An abstract {@link org.apache.flink.client.program.PackagedProgramRetriever + * PackagedProgramRetriever} which creates the {@link + * org.apache.flink.client.program.PackagedProgram PackagedProgram} containing the user's {@code + * main()} from a class on the class path. + */ +@Internal +public abstract class AbstractPackagedProgramRetriever implements PackagedProgramRetriever { + + @Nonnull protected final String[] programArguments; Review comment: nit: I think we could remove the `@Nonnull` annotation since we assume that all the fields are non null by default. Refer to the [check style](https://flink.apache.org/contributing/code-style-and-quality-common.html#nullability-of-the-mutable-parts) for more information. ########## File path: flink-clients/src/main/java/org/apache/flink/client/deployment/application/JarFilePackagedProgramRetriever.java ########## @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.client.deployment.application; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.client.program.PackagedProgram; +import org.apache.flink.client.program.ProgramInvocationException; +import org.apache.flink.configuration.Configuration; +import org.apache.flink.util.FlinkException; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; + +/** + * A jar file {@link org.apache.flink.client.program.PackagedProgramRetriever + * PackagedProgramRetriever} which creates the {@link + * org.apache.flink.client.program.PackagedProgram PackagedProgram} with the specified jar file. + */ +@Internal +public class JarFilePackagedProgramRetriever extends AbstractPackagedProgramRetriever { + + @Nullable private final String jobClassName; + + @Nullable private final File jarFile; Review comment: I do not think the jarFile could be `null` here. ########## File path: flink-clients/src/test/java/org/apache/flink/client/deployment/application/PackagedProgramRetrieverTestBase.java ########## @@ -0,0 +1,150 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.client.deployment.application; + +import org.apache.flink.api.dag.Pipeline; +import org.apache.flink.client.deployment.executors.PipelineExecutorUtils; +import org.apache.flink.client.program.PackagedProgram; +import org.apache.flink.client.program.PackagedProgramRetriever; +import org.apache.flink.client.program.PackagedProgramUtils; +import org.apache.flink.client.program.ProgramInvocationException; +import org.apache.flink.client.testjar.TestJobInfo; +import org.apache.flink.configuration.ConfigUtils; +import org.apache.flink.configuration.Configuration; +import org.apache.flink.configuration.CoreOptions; +import org.apache.flink.configuration.PipelineOptions; +import org.apache.flink.runtime.jobgraph.JobGraph; +import org.apache.flink.util.FileUtils; +import org.apache.flink.util.FlinkException; +import org.apache.flink.util.TestLogger; +import org.apache.flink.util.function.FunctionUtils; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; +import java.util.stream.Stream; + +/** Abstract test base for the {@link PackagedProgramRetriever}. */ +public abstract class PackagedProgramRetrieverTestBase extends TestLogger { + + @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @ClassRule public static final TemporaryFolder JOB_DIRS = new TemporaryFolder(); + + static final String[] PROGRAM_ARGUMENTS = {"--arg", "suffix"}; + + /* + * The directory structure used to test + * + * userDirHasEntryClass/ + * |_jarWithEntryClass + * |_jarWithoutEntryClass + * |_textFile + * + * userDirHasNotEntryClass/ + * |_jarWithoutEntryClass + * |_textFile + */ + + static final Collection<URL> EXPECTED_URLS = new ArrayList<>(); + + static File userDirHasEntryClass; + + static File userDirHasNotEntryClass; + + @BeforeClass + public static void init() throws IOException { + final String textFileName = "test.txt"; + final String userDirHasEntryClassName = "_test_user_dir_has_entry_class"; + final String userDirHasNotEntryClassName = "_test_user_dir_has_not_entry_class"; + + userDirHasEntryClass = JOB_DIRS.newFolder(userDirHasEntryClassName); + userDirHasNotEntryClass = JOB_DIRS.newFolder(userDirHasNotEntryClassName); + + final Path userJarPath = + userDirHasEntryClass.toPath().resolve(TestJobInfo.JOB_JAR_PATH.toFile().getName()); + final Path userLibJarPath = + userDirHasEntryClass + .toPath() + .resolve(TestJobInfo.JOB_LIB_JAR_PATH.toFile().getName()); + + // create files + Files.copy(TestJobInfo.JOB_JAR_PATH, userJarPath); + Files.copy(TestJobInfo.JOB_LIB_JAR_PATH, userLibJarPath); + + Files.createFile(userDirHasEntryClass.toPath().resolve(textFileName)); + Files.copy( + TestJobInfo.JOB_LIB_JAR_PATH, + userDirHasNotEntryClass + .toPath() + .resolve(TestJobInfo.JOB_LIB_JAR_PATH.toFile().getName())); + + Files.createFile(userDirHasNotEntryClass.toPath().resolve(textFileName)); + + final Path workingDirectory = FileUtils.getCurrentWorkingDirectory(); + Stream.of(userJarPath, userLibJarPath) + .map(path -> FileUtils.relativizePath(workingDirectory, path)) + .map(FunctionUtils.uncheckedFunction(FileUtils::toURL)) + .forEach(EXPECTED_URLS::add); + } + + @AfterClass + public static void clean() { + if (!EXPECTED_URLS.isEmpty()) { + EXPECTED_URLS.clear(); + } + } + + abstract void testJobGraphRetrieval() + throws FlinkException, ProgramInvocationException, IOException; + + JobGraph retrieveJobGraph( Review comment: Could be `protected`. ########## File path: flink-clients/src/test/java/org/apache/flink/client/deployment/application/JarFilePackagedProgramRetrieverTest.java ########## @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.client.deployment.application; + +import org.apache.flink.client.program.PackagedProgramRetriever; +import org.apache.flink.client.program.ProgramInvocationException; +import org.apache.flink.client.testjar.TestJob; +import org.apache.flink.configuration.Configuration; +import org.apache.flink.core.fs.Path; +import org.apache.flink.runtime.jobgraph.JobGraph; +import org.apache.flink.util.FlinkException; + +import org.junit.Test; + +import java.io.File; +import java.io.IOException; +import java.net.URL; +import java.util.stream.Collectors; + +import static org.apache.flink.client.deployment.application.PackagedProgramRetrieverAdapter.newBuilder; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +/** Tests for the {@link JarFilePackagedProgramRetriever}. */ +public class JarFilePackagedProgramRetrieverTest extends PackagedProgramRetrieverTestBase { + + @Test Review comment: I do not suggest to verify multiple behaviors in one test. Why do we directly make `testRetrieveFromJarFileWithoutUserLib` and `testRetrieveFromJarFileWithUserLib` as `@Test`. ########## File path: flink-clients/src/main/java/org/apache/flink/client/deployment/application/AbstractPackagedProgramRetriever.java ########## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.client.deployment.application; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.client.program.PackagedProgram; +import org.apache.flink.client.program.PackagedProgramRetriever; +import org.apache.flink.client.program.ProgramInvocationException; +import org.apache.flink.configuration.Configuration; +import org.apache.flink.util.FileUtils; +import org.apache.flink.util.FlinkException; +import org.apache.flink.util.function.FunctionUtils; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import java.io.File; +import java.io.IOException; +import java.net.URL; +import java.nio.file.Path; +import java.util.Collection; +import java.util.Collections; +import java.util.stream.Collectors; + +import static java.util.Objects.requireNonNull; + +/** + * An abstract {@link org.apache.flink.client.program.PackagedProgramRetriever + * PackagedProgramRetriever} which creates the {@link + * org.apache.flink.client.program.PackagedProgram PackagedProgram} containing the user's {@code + * main()} from a class on the class path. + */ +@Internal +public abstract class AbstractPackagedProgramRetriever implements PackagedProgramRetriever { + + @Nonnull protected final String[] programArguments; + + @Nonnull protected final Configuration configuration; + + /** User class paths in relative form to the working directory. */ + @Nonnull protected final Collection<URL> userClassPaths; + + AbstractPackagedProgramRetriever( + @Nonnull String[] programArguments, + @Nonnull Configuration configuration, + @Nullable File userLibDirectory) + throws IOException { + this.programArguments = requireNonNull(programArguments, "programArguments"); Review comment: Maybe we could `Preconditions.checkNotNull` instead of `java.util.Objects.requireNonNull`. ########## File path: flink-container/src/main/java/org/apache/flink/container/entrypoint/StandaloneApplicationClusterEntryPoint.java ########## @@ -101,22 +104,28 @@ static Configuration loadConfigurationFromClusterConfig( } private static PackagedProgram getPackagedProgram( - final StandaloneApplicationClusterConfiguration clusterConfiguration) + final StandaloneApplicationClusterConfiguration clusterConfiguration, + final Configuration configuration) throws IOException, FlinkException { final PackagedProgramRetriever programRetriever = getPackagedProgramRetriever( - clusterConfiguration.getArgs(), clusterConfiguration.getJobClassName()); + configuration, + clusterConfiguration.getArgs(), + clusterConfiguration.getJobClassName()); return programRetriever.getPackagedProgram(); } private static PackagedProgramRetriever getPackagedProgramRetriever( - final String[] programArguments, @Nullable final String jobClassName) + final Configuration configuration, + final String[] programArguments, + @Nullable final String jobClassName) throws IOException { final File userLibDir = ClusterEntrypointUtils.tryFindUserLibDirectory().orElse(null); - final ClassPathPackagedProgramRetriever.Builder retrieverBuilder = - ClassPathPackagedProgramRetriever.newBuilder(programArguments) - .setUserLibDirectory(userLibDir) - .setJobClassName(jobClassName); + final Builder retrieverBuilder = + newBuilder(programArguments) Review comment: I suggest to use `PackagedProgramRetrieverAdapter.newBuilder`. Otherwise, it is really confusing where `newBuilder` comes from. We have some other similar cases in other classes. ########## File path: flink-clients/src/test/java/org/apache/flink/client/deployment/application/PackagedProgramRetrieverTestBase.java ########## @@ -0,0 +1,150 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.client.deployment.application; + +import org.apache.flink.api.dag.Pipeline; +import org.apache.flink.client.deployment.executors.PipelineExecutorUtils; +import org.apache.flink.client.program.PackagedProgram; +import org.apache.flink.client.program.PackagedProgramRetriever; +import org.apache.flink.client.program.PackagedProgramUtils; +import org.apache.flink.client.program.ProgramInvocationException; +import org.apache.flink.client.testjar.TestJobInfo; +import org.apache.flink.configuration.ConfigUtils; +import org.apache.flink.configuration.Configuration; +import org.apache.flink.configuration.CoreOptions; +import org.apache.flink.configuration.PipelineOptions; +import org.apache.flink.runtime.jobgraph.JobGraph; +import org.apache.flink.util.FileUtils; +import org.apache.flink.util.FlinkException; +import org.apache.flink.util.TestLogger; +import org.apache.flink.util.function.FunctionUtils; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; +import java.util.stream.Stream; + +/** Abstract test base for the {@link PackagedProgramRetriever}. */ +public abstract class PackagedProgramRetrieverTestBase extends TestLogger { + + @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @ClassRule public static final TemporaryFolder JOB_DIRS = new TemporaryFolder(); + + static final String[] PROGRAM_ARGUMENTS = {"--arg", "suffix"}; + + /* + * The directory structure used to test + * + * userDirHasEntryClass/ + * |_jarWithEntryClass + * |_jarWithoutEntryClass + * |_textFile + * + * userDirHasNotEntryClass/ + * |_jarWithoutEntryClass + * |_textFile + */ + + static final Collection<URL> EXPECTED_URLS = new ArrayList<>(); + + static File userDirHasEntryClass; + + static File userDirHasNotEntryClass; + + @BeforeClass + public static void init() throws IOException { + final String textFileName = "test.txt"; + final String userDirHasEntryClassName = "_test_user_dir_has_entry_class"; + final String userDirHasNotEntryClassName = "_test_user_dir_has_not_entry_class"; + + userDirHasEntryClass = JOB_DIRS.newFolder(userDirHasEntryClassName); + userDirHasNotEntryClass = JOB_DIRS.newFolder(userDirHasNotEntryClassName); + + final Path userJarPath = + userDirHasEntryClass.toPath().resolve(TestJobInfo.JOB_JAR_PATH.toFile().getName()); + final Path userLibJarPath = + userDirHasEntryClass + .toPath() + .resolve(TestJobInfo.JOB_LIB_JAR_PATH.toFile().getName()); + + // create files + Files.copy(TestJobInfo.JOB_JAR_PATH, userJarPath); + Files.copy(TestJobInfo.JOB_LIB_JAR_PATH, userLibJarPath); + + Files.createFile(userDirHasEntryClass.toPath().resolve(textFileName)); + Files.copy( + TestJobInfo.JOB_LIB_JAR_PATH, + userDirHasNotEntryClass + .toPath() + .resolve(TestJobInfo.JOB_LIB_JAR_PATH.toFile().getName())); + + Files.createFile(userDirHasNotEntryClass.toPath().resolve(textFileName)); + + final Path workingDirectory = FileUtils.getCurrentWorkingDirectory(); + Stream.of(userJarPath, userLibJarPath) + .map(path -> FileUtils.relativizePath(workingDirectory, path)) + .map(FunctionUtils.uncheckedFunction(FileUtils::toURL)) + .forEach(EXPECTED_URLS::add); + } + + @AfterClass + public static void clean() { + if (!EXPECTED_URLS.isEmpty()) { + EXPECTED_URLS.clear(); + } + } + + abstract void testJobGraphRetrieval() Review comment: I do not fully understand why do we have this `abstract` method. ########## File path: flink-clients/src/test/java/org/apache/flink/client/testjar/TestJobInfo.java ########## @@ -25,8 +25,6 @@ public class TestJobInfo { public static final String JOB_CLASS = "org.apache.flink.client.testjar.TestUserClassLoaderJob"; - public static final String JOB_LIB_CLASS = Review comment: Please split this unrelated change into a separate `hotfix` commit. ########## File path: flink-clients/src/main/java/org/apache/flink/client/deployment/application/JarFilePackagedProgramRetriever.java ########## @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.client.deployment.application; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.client.program.PackagedProgram; +import org.apache.flink.client.program.ProgramInvocationException; +import org.apache.flink.configuration.Configuration; +import org.apache.flink.util.FlinkException; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; + +/** + * A jar file {@link org.apache.flink.client.program.PackagedProgramRetriever + * PackagedProgramRetriever} which creates the {@link + * org.apache.flink.client.program.PackagedProgram PackagedProgram} with the specified jar file. + */ +@Internal +public class JarFilePackagedProgramRetriever extends AbstractPackagedProgramRetriever { + + @Nullable private final String jobClassName; + + @Nullable private final File jarFile; + + protected JarFilePackagedProgramRetriever( + @Nonnull String[] programArguments, + @Nonnull Configuration configuration, + @Nullable String jobClassName, + @Nullable File userLibDirectory, + @Nullable File jarFile) + throws IOException { + super(programArguments, configuration, userLibDirectory); + this.jobClassName = jobClassName; + this.jarFile = jarFile; Review comment: non-null check. ########## File path: flink-clients/src/test/java/org/apache/flink/client/deployment/application/PackagedProgramRetrieverAdapterTest.java ########## @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.client.deployment.application; + +import org.apache.flink.client.program.PackagedProgramRetriever; +import org.apache.flink.client.testjar.TestJob; +import org.apache.flink.util.TestLogger; + +import org.junit.Test; + +import java.io.File; +import java.io.IOException; + +import static org.apache.flink.client.deployment.application.PackagedProgramRetrieverAdapter.newBuilder; +import static org.junit.Assert.assertTrue; + +/** Tests for the {@link PackagedProgramRetrieverAdapter}. */ +public class PackagedProgramRetrieverAdapterTest extends TestLogger { + + @Test + public void testBuildPackagedProgramRetriever() throws IOException { + final String[] programArguments = {"--arg", "suffix"}; + PackagedProgramRetriever retrieverUnderTest = + newBuilder(programArguments) + .setJobClassName(TestJob.class.getCanonicalName()) + .build(); + assertTrue(retrieverUnderTest instanceof ClassPathPackagedProgramRetriever); Review comment: We could separate these adapter logics(jar file, python or nothing) into different tests. After then when one test failed, we could easily find out which is the root cause. For python, we could have two test. One is for python entrypoint class setting, and another is from args. ########## File path: flink-clients/src/test/java/org/apache/flink/client/deployment/application/ClassPathPackagedProgramRetrieverTest.java ########## @@ -312,87 +205,64 @@ public void testRetrieveCorrectUserClasspathsWithSpecifiedEntryClass() assertThat( jobGraph.getClasspaths().stream().map(URL::toString).collect(Collectors.toList()), - containsInAnyOrder(expectedURLs.stream().map(URL::toString).toArray())); + containsInAnyOrder(EXPECTED_URLS.stream().map(URL::toString).toArray())); } @Test - public void testRetrieveFromJarFileWithoutUserLib() - throws IOException, FlinkException, ProgramInvocationException { - final File testJar = TestJob.getTestJobJar(); - final ClassPathPackagedProgramRetriever retrieverUnderTest = - ClassPathPackagedProgramRetriever.newBuilder(PROGRAM_ARGUMENTS) - .setJarFile(testJar) - .build(); - final JobGraph jobGraph = retrieveJobGraph(retrieverUnderTest, new Configuration()); + public void testGetPackagedProgramWithConfiguration() throws IOException, FlinkException { Review comment: This should also be tested for JarFilePackagedProgramRetriever and PythonBasedPackagedProgramRetriever. ########## File path: flink-clients/src/test/java/org/apache/flink/client/deployment/application/ClassPathPackagedProgramRetrieverTest.java ########## @@ -285,25 +148,55 @@ public void testJobGraphRetrievalFailIfDoesNotFindTheEntryClassInTheJobDir() } @Test - public void testRetrieveCorrectUserClasspathsWithoutSpecifiedEntryClass() + public void testSavepointRestoreSettings() Review comment: This should also be tested for `JarFilePackagedProgramRetriever` and `PythonBasedPackagedProgramRetriever`. Maybe we could add it to the `PackagedProgramRetrieverTestBase`. ---------------------------------------------------------------- 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: [email protected]
