flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GJL <...@git.apache.org>
Subject [GitHub] flink pull request #5226: [FLINK-8340] [flip6] Remove passing of Configurati...
Date Fri, 05 Jan 2018 14:12:47 GMT
Github user GJL commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5226#discussion_r159883795
  
    --- Diff: flink-clients/src/test/java/org/apache/flink/client/cli/CliFrontendPackageProgramTest.java
---
    @@ -60,220 +59,178 @@ public static void init() {
     	}
     
     	@Test
    -	public void testNonExistingJarFile() {
    -		try {
    -			CliFrontend frontend = new CliFrontend(
    -				new Configuration(),
    -				Collections.singletonList(new DefaultCLI()),
    -				CliFrontendTestUtils.getConfigDir());
    +	public void testNonExistingJarFile() throws Exception {
    +		Configuration configuration = new Configuration();
    +		CliFrontend frontend = new CliFrontend(
    +			configuration,
    +			Collections.singletonList(new DefaultCLI(configuration)));
     
    -			ProgramOptions options = mock(ProgramOptions.class);
    -			when(options.getJarFilePath()).thenReturn("/some/none/existing/path");
    +		ProgramOptions options = mock(ProgramOptions.class);
    +		when(options.getJarFilePath()).thenReturn("/some/none/existing/path");
     
    -			try {
    -				frontend.buildProgram(options);
    -				fail("should throw an exception");
    -			}
    -			catch (FileNotFoundException e) {
    -				// that's what we want
    -			}
    +		try {
    +			frontend.buildProgram(options);
    +			fail("should throw an exception");
     		}
    -		catch (Exception e) {
    -			e.printStackTrace();
    -			fail(e.getMessage());
    +		catch (FileNotFoundException e) {
    +			// that's what we want
     		}
     	}
     
     	@Test
    -	public void testFileNotJarFile() {
    -		try {
    -			CliFrontend frontend = new CliFrontend(
    -				new Configuration(),
    -				Collections.singletonList(new DefaultCLI()),
    -				CliFrontendTestUtils.getConfigDir());
    +	public void testFileNotJarFile() throws Exception {
    +		Configuration configuration = new Configuration();
    +		CliFrontend frontend = new CliFrontend(
    +			configuration,
    +			Collections.singletonList(new DefaultCLI(configuration)));
     
    -			ProgramOptions options = mock(ProgramOptions.class);
    -			when(options.getJarFilePath()).thenReturn(getNonJarFilePath());
    +		ProgramOptions options = mock(ProgramOptions.class);
    +		when(options.getJarFilePath()).thenReturn(getNonJarFilePath());
     
    -			try {
    -				frontend.buildProgram(options);
    -				fail("should throw an exception");
    -			}
    -			catch (ProgramInvocationException e) {
    -				// that's what we want
    -			}
    +		try {
    +			frontend.buildProgram(options);
    +			fail("should throw an exception");
     		}
    -		catch (Exception e) {
    -			e.printStackTrace();
    -			fail(e.getMessage());
    +		catch (ProgramInvocationException e) {
    +			// that's what we want
     		}
     	}
     
     	@Test
    -	public void testVariantWithExplicitJarAndArgumentsOption() {
    -		try {
    -			String[] arguments = {
    -					"--classpath", "file:///tmp/foo",
    -					"--classpath", "file:///tmp/bar",
    -					"-j", getTestJarPath(),
    -					"-a", "--debug", "true", "arg1", "arg2" };
    -			URL[] classpath = new URL[] { new URL("file:///tmp/foo"), new URL("file:///tmp/bar")
};
    -			String[] reducedArguments = new String[] {"--debug", "true", "arg1", "arg2"};
    -
    -			RunOptions options = CliFrontendParser.parseRunCommand(arguments);
    -			assertEquals(getTestJarPath(), options.getJarFilePath());
    -			assertArrayEquals(classpath, options.getClasspaths().toArray());
    -			assertArrayEquals(reducedArguments, options.getProgramArgs());
    -
    -			CliFrontend frontend = new CliFrontend(
    -				new Configuration(),
    -				Collections.singletonList(new DefaultCLI()),
    -				CliFrontendTestUtils.getConfigDir());
    -			PackagedProgram prog = frontend.buildProgram(options);
    +	public void testVariantWithExplicitJarAndArgumentsOption() throws Exception {
    +		String[] arguments = {
    +				"--classpath", "file:///tmp/foo",
    +				"--classpath", "file:///tmp/bar",
    +				"-j", getTestJarPath(),
    +				"-a", "--debug", "true", "arg1", "arg2" };
    +		URL[] classpath = new URL[] { new URL("file:///tmp/foo"), new URL("file:///tmp/bar")
};
    +		String[] reducedArguments = new String[] {"--debug", "true", "arg1", "arg2"};
    +
    +		RunOptions options = CliFrontendParser.parseRunCommand(arguments);
    +		assertEquals(getTestJarPath(), options.getJarFilePath());
    +		assertArrayEquals(classpath, options.getClasspaths().toArray());
    +		assertArrayEquals(reducedArguments, options.getProgramArgs());
    +
    +		Configuration configuration = new Configuration();
    +		CliFrontend frontend = new CliFrontend(
    +			configuration,
    +			Collections.singletonList(new DefaultCLI(configuration)));
    +		PackagedProgram prog = frontend.buildProgram(options);
     
    -			Assert.assertArrayEquals(reducedArguments, prog.getArguments());
    -			Assert.assertEquals(TEST_JAR_MAIN_CLASS, prog.getMainClassName());
    -		}
    -		catch (Exception e) {
    -			e.printStackTrace();
    -			fail(e.getMessage());
    -		}
    +		Assert.assertArrayEquals(reducedArguments, prog.getArguments());
    +		Assert.assertEquals(TEST_JAR_MAIN_CLASS, prog.getMainClassName());
     	}
     
     	@Test
    -	public void testVariantWithExplicitJarAndNoArgumentsOption() {
    -		try {
    -			String[] arguments = {
    -					"--classpath", "file:///tmp/foo",
    -					"--classpath", "file:///tmp/bar",
    -					"-j", getTestJarPath(),
    -					"--debug", "true", "arg1", "arg2" };
    -			URL[] classpath = new URL[] { new URL("file:///tmp/foo"), new URL("file:///tmp/bar")
};
    -			String[] reducedArguments = new String[] {"--debug", "true", "arg1", "arg2"};
    -
    -			RunOptions options = CliFrontendParser.parseRunCommand(arguments);
    -			assertEquals(getTestJarPath(), options.getJarFilePath());
    -			assertArrayEquals(classpath, options.getClasspaths().toArray());
    -			assertArrayEquals(reducedArguments, options.getProgramArgs());
    -
    -			CliFrontend frontend = new CliFrontend(
    -				new Configuration(),
    -				Collections.singletonList(new DefaultCLI()),
    -				CliFrontendTestUtils.getConfigDir());
    +	public void testVariantWithExplicitJarAndNoArgumentsOption() throws Exception {
    +		String[] arguments = {
    +				"--classpath", "file:///tmp/foo",
    +				"--classpath", "file:///tmp/bar",
    +				"-j", getTestJarPath(),
    +				"--debug", "true", "arg1", "arg2" };
    +		URL[] classpath = new URL[] { new URL("file:///tmp/foo"), new URL("file:///tmp/bar")
};
    +		String[] reducedArguments = new String[] {"--debug", "true", "arg1", "arg2"};
    +
    +		RunOptions options = CliFrontendParser.parseRunCommand(arguments);
    +		assertEquals(getTestJarPath(), options.getJarFilePath());
    +		assertArrayEquals(classpath, options.getClasspaths().toArray());
    +		assertArrayEquals(reducedArguments, options.getProgramArgs());
    +
    +		Configuration configuration = new Configuration();
    +		CliFrontend frontend = new CliFrontend(
    +			configuration,
    +			Collections.singletonList(new DefaultCLI(configuration)));
     
    -			PackagedProgram prog = frontend.buildProgram(options);
    +		PackagedProgram prog = frontend.buildProgram(options);
     
    -			Assert.assertArrayEquals(reducedArguments, prog.getArguments());
    -			Assert.assertEquals(TEST_JAR_MAIN_CLASS, prog.getMainClassName());
    -		}
    -		catch (Exception e) {
    -			e.printStackTrace();
    -			fail(e.getMessage());
    -		}
    +		Assert.assertArrayEquals(reducedArguments, prog.getArguments());
    +		Assert.assertEquals(TEST_JAR_MAIN_CLASS, prog.getMainClassName());
     	}
     
     	@Test
    -	public void testValidVariantWithNoJarAndNoArgumentsOption() {
    -		try {
    -			String[] arguments = {
    -					"--classpath", "file:///tmp/foo",
    -					"--classpath", "file:///tmp/bar",
    -					getTestJarPath(),
    -					"--debug", "true", "arg1", "arg2" };
    -			URL[] classpath = new URL[] { new URL("file:///tmp/foo"), new URL("file:///tmp/bar")
};
    -			String[] reducedArguments = {"--debug", "true", "arg1", "arg2"};
    -
    -			RunOptions options = CliFrontendParser.parseRunCommand(arguments);
    -			assertEquals(getTestJarPath(), options.getJarFilePath());
    -			assertArrayEquals(classpath, options.getClasspaths().toArray());
    -			assertArrayEquals(reducedArguments, options.getProgramArgs());
    -
    -			CliFrontend frontend = new CliFrontend(
    -				new Configuration(),
    -				Collections.singletonList(new DefaultCLI()),
    -				CliFrontendTestUtils.getConfigDir());
    +	public void testValidVariantWithNoJarAndNoArgumentsOption() throws Exception {
    +		String[] arguments = {
    +				"--classpath", "file:///tmp/foo",
    +				"--classpath", "file:///tmp/bar",
    +				getTestJarPath(),
    +				"--debug", "true", "arg1", "arg2" };
    +		URL[] classpath = new URL[] { new URL("file:///tmp/foo"), new URL("file:///tmp/bar")
};
    +		String[] reducedArguments = {"--debug", "true", "arg1", "arg2"};
    +
    +		RunOptions options = CliFrontendParser.parseRunCommand(arguments);
    +		assertEquals(getTestJarPath(), options.getJarFilePath());
    +		assertArrayEquals(classpath, options.getClasspaths().toArray());
    +		assertArrayEquals(reducedArguments, options.getProgramArgs());
    +
    +		Configuration configuration = new Configuration();
    +		CliFrontend frontend = new CliFrontend(
    +			configuration,
    +			Collections.singletonList(new DefaultCLI(configuration)));
     
    -			PackagedProgram prog = frontend.buildProgram(options);
    +		PackagedProgram prog = frontend.buildProgram(options);
     
    -			Assert.assertArrayEquals(reducedArguments, prog.getArguments());
    -			Assert.assertEquals(TEST_JAR_MAIN_CLASS, prog.getMainClassName());
    -		}
    -		catch (Exception e) {
    -			e.printStackTrace();
    -			fail(e.getMessage());
    -		}
    +		Assert.assertArrayEquals(reducedArguments, prog.getArguments());
    +		Assert.assertEquals(TEST_JAR_MAIN_CLASS, prog.getMainClassName());
     	}
     
     	@Test(expected = CliArgsException.class)
     	public void testNoJarNoArgumentsAtAll() throws Exception {
    +		Configuration configuration = new Configuration();
     		CliFrontend frontend = new CliFrontend(
    -			new Configuration(),
    -			Collections.singletonList(new DefaultCLI()),
    -			CliFrontendTestUtils.getConfigDir());
    +			configuration,
    +			Collections.singletonList(new DefaultCLI(configuration)));
     		assertTrue(frontend.run(new String[0]) != 0);
     
     		fail("Should have failed.");
    --- End diff --
    
    Failing explicitly isn't required if you use `@Test(expected = CliArgsException.class)`.
For example
    ```
            @Test(expected = CliArgsException.class)
    	public void testNoJarNoArgumentsAtAll() throws Exception {
    		try {
    			Configuration configuration = new Configuration();
    			CliFrontend frontend = new CliFrontend(
    				configuration,
    				Collections.singletonList(new DefaultCLI(configuration)));
    			frontend.run(new String[0]);
    		} catch (Exception e) {
    		}
    	}
    ```
    Fails with
    ```
    java.lang.AssertionError: Expected exception: org.apache.flink.client.cli.CliArgsException
    
    	at org.junit.internal.runners.statements.ExpectException.evaluate(ExpectException.java:32)
    	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
    	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
    	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
    	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
    	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
    	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
    
    ```
      


---

Mime
View raw message