akomakom commented on a change in pull request #285: URL: https://github.com/apache/maven-surefire/pull/285#discussion_r414681374
########## File path: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoToolchainsTest.java ########## @@ -37,33 +37,91 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static junit.framework.TestCase.assertNull; import static org.fest.assertions.Assertions.assertThat; -import static org.fest.assertions.Fail.fail; import static org.powermock.api.mockito.PowerMockito.mock; +import static org.powermock.api.mockito.PowerMockito.mockStatic; +import static org.powermock.api.mockito.PowerMockito.when; import static org.powermock.reflect.Whitebox.invokeMethod; /** - * Test for {@link AbstractSurefireMojo}. + * Test for {@link AbstractSurefireMojo}. jdkToolchain parameter */ @RunWith( PowerMockRunner.class ) @PrepareForTest( {AbstractSurefireMojo.class, ResolvePathsRequest.class, ReflectionUtils.class} ) @PowerMockIgnore( {"org.jacoco.agent.rt.*", "com.vladium.emma.rt.*"} ) public class AbstractSurefireMojoToolchainsTest { + @Test + public void shouldCallMaven33xMethodWhenSpecSet() throws Exception + { + AbstractSurefireMojoTest.Mojo mojo = new AbstractSurefireMojoTest.Mojo(); + Toolchain expectedFromMaven33Method = mock( Toolchain.class ); + MockToolchainManager toolchainManager = new MockToolchainManager( null, null ); + mojo.setToolchainManager( toolchainManager ); + mojo.setJdkToolchain( singletonMap( "version", "1.8" ) ); + + mockStatic( AbstractSurefireMojo.class ); + when( + AbstractSurefireMojo.class, + "getToolchainMaven33x", + ToolchainManager.class, + toolchainManager, + mojo.getSession(), mojo.getJdkToolchain() ).thenReturn( expectedFromMaven33Method ); + Toolchain actual = invokeMethod( mojo, "getToolchain" ); + assertThat( actual ) + .isSameAs( expectedFromMaven33Method ); + } + + @Test + public void shouldFallthroughToBuildContextWhenNoSpecSet() throws Exception + { + AbstractSurefireMojoTest.Mojo mojo = new AbstractSurefireMojoTest.Mojo(); + Toolchain expectedFromContext = mock( Toolchain.class ); + Toolchain expectedFromSpec = mock( Toolchain.class ); //ensure it still behaves correctly even if not null + mojo.setToolchainManager( new MockToolchainManager( expectedFromSpec, expectedFromContext ) ); + Toolchain actual = invokeMethod( mojo, "getToolchain" ); Review comment: Because jdkToolchain is null, so `getToolchainMaven33x()` never called. I added some javadoc to the test for anyone trying to figure this out in the future. ########## File path: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoToolchainsTest.java ########## @@ -37,33 +37,91 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static junit.framework.TestCase.assertNull; import static org.fest.assertions.Assertions.assertThat; -import static org.fest.assertions.Fail.fail; import static org.powermock.api.mockito.PowerMockito.mock; +import static org.powermock.api.mockito.PowerMockito.mockStatic; +import static org.powermock.api.mockito.PowerMockito.when; import static org.powermock.reflect.Whitebox.invokeMethod; /** - * Test for {@link AbstractSurefireMojo}. + * Test for {@link AbstractSurefireMojo}. jdkToolchain parameter */ @RunWith( PowerMockRunner.class ) @PrepareForTest( {AbstractSurefireMojo.class, ResolvePathsRequest.class, ReflectionUtils.class} ) @PowerMockIgnore( {"org.jacoco.agent.rt.*", "com.vladium.emma.rt.*"} ) public class AbstractSurefireMojoToolchainsTest { + @Test + public void shouldCallMaven33xMethodWhenSpecSet() throws Exception + { + AbstractSurefireMojoTest.Mojo mojo = new AbstractSurefireMojoTest.Mojo(); + Toolchain expectedFromMaven33Method = mock( Toolchain.class ); + MockToolchainManager toolchainManager = new MockToolchainManager( null, null ); + mojo.setToolchainManager( toolchainManager ); + mojo.setJdkToolchain( singletonMap( "version", "1.8" ) ); + + mockStatic( AbstractSurefireMojo.class ); + when( + AbstractSurefireMojo.class, + "getToolchainMaven33x", + ToolchainManager.class, + toolchainManager, + mojo.getSession(), mojo.getJdkToolchain() ).thenReturn( expectedFromMaven33Method ); + Toolchain actual = invokeMethod( mojo, "getToolchain" ); + assertThat( actual ) + .isSameAs( expectedFromMaven33Method ); + } + + @Test + public void shouldFallthroughToBuildContextWhenNoSpecSet() throws Exception + { + AbstractSurefireMojoTest.Mojo mojo = new AbstractSurefireMojoTest.Mojo(); + Toolchain expectedFromContext = mock( Toolchain.class ); + Toolchain expectedFromSpec = mock( Toolchain.class ); //ensure it still behaves correctly even if not null + mojo.setToolchainManager( new MockToolchainManager( expectedFromSpec, expectedFromContext ) ); + Toolchain actual = invokeMethod( mojo, "getToolchain" ); Review comment: Because jdkToolchain is null, so `getToolchainMaven33x()` is never called. I added some javadoc to the test for anyone trying to figure this out in the future. ---------------------------------------------------------------- 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: us...@infra.apache.org