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


Reply via email to