From 25aee7164e9d460028ac5b22098628404f841e42 Mon Sep 17 00:00:00 2001
From: Khalid Shakir <kshakir@broadinstitute.org>
Date: Thu, 30 Jan 2014 17:47:54 +0800
Subject: [PATCH] Fixed missing "mvn" command execution in ant-bridge. Added
 pom.xml workarounds for duplicate classpath error, due to gatk-framework
 dependency containing required BaseTest, and jarred
 *UnitTest/*IntegrationTest classes that also exist as files under
 target/test-classes.

---
 ant-bridge.sh                   |  6 ++--
 pom.xml                         | 23 +++++++--------
 public/external-example/pom.xml |  5 +---
 public/gatk-framework/pom.xml   | 34 ++++++++++++++++++++++
 public/package-tests/pom.xml    | 51 +++++++++++++++++++++++++++------
 public/sting-root/pom.xml       |  7 +----
 6 files changed, 93 insertions(+), 33 deletions(-)

diff --git a/ant-bridge.sh b/ant-bridge.sh
index f374cd99e..e9af3e749 100755
--- a/ant-bridge.sh
+++ b/ant-bridge.sh
@@ -87,7 +87,7 @@ for arg in "${@}" ; do
         # TODO: This runs *all* commit tests, including the few on Queue.
         elif [ "${arg}" = "gatkfull.binary.release.tests" ] ; then
             local_repo="sitetemprepo"
-            mvn_args="install -Dmaven.repo.local=${local_repo} && verify"
+            mvn_args="install -Dmaven.repo.local=${local_repo} && mvn verify"
             mvn_args="${mvn_args} -Dmaven.repo.local=${local_repo} -Dmaven.javadoc.skip=true"
             mvn_args="${mvn_args} -Dsting.generate-gatk-extensions.skipped=true"
             mvn_args="${mvn_args} -Dsting.jar.phase=none -Dsting.unpack.phase=none -Dsting.shade.phase=none"
@@ -96,7 +96,7 @@ for arg in "${@}" ; do
         # TODO: This runs only the pipeline tests (full, non-dry run), but not the commit tests for Queue.
         elif [ "${arg}" = "queuefull.binary.release.tests" ] ; then
             local_repo="sitetemprepo"
-            mvn_args="install -Dmaven.repo.local=${local_repo} && verify"
+            mvn_args="install -Dmaven.repo.local=${local_repo} && mvn verify"
             mvn_args="${mvn_args} -Dmaven.repo.local=${local_repo} -Dmaven.javadoc.skip=true"
             mvn_args="${mvn_args} -Dsting.generate-gatk-extensions.skipped=true"
             mvn_args="${mvn_args} -Dsting.jar.phase=none -Dsting.unpack.phase=none -Dsting.shade.phase=none"
@@ -153,6 +153,6 @@ if [ "${unknown_args}" != "" ]; then
 else
     echo "Equivalent maven command"
     echo "${mvn_cmd}${mvn_properties}${post_script}"
-    sh -c "${mvn_cmd}${mvn_properties}${post_script}"
+#    sh -c "${mvn_cmd}${mvn_properties}${post_script}"
 
 fi
diff --git a/pom.xml b/pom.xml
index 787bd5d31..2100556ed 100644
--- a/pom.xml
+++ b/pom.xml
@@ -508,6 +508,9 @@
                         </properties>
                     </configuration>
                     <executions>
+                        <!--
+                        TODO: see description of disable.gatk-framework.test.dependency in gatk-framework/pom.xml
+                        -->
                         <execution>
                             <id>package-unittests</id>
                             <goals>
@@ -517,11 +520,10 @@
                                 <goals>
                                     <goal>test</goal>
                                 </goals>
-                                <profiles>
-                                    <profile>unittests</profile>
-                                </profiles>
                                 <skipInvocation>${sting.packageunittests.skipped}</skipInvocation>
                                 <properties>
+                                    <unittests.profile.enabled>true</unittests.profile.enabled>
+                                    <disable.gatk-framework.test.dependency>false</disable.gatk-framework.test.dependency>
                                     <sting.packageunittests.skipped>${sting.packageunittests.skipped}</sting.packageunittests.skipped>
                                 </properties>
                             </configuration>
@@ -536,11 +538,10 @@
                                 <goals>
                                     <goal>verify</goal>
                                 </goals>
-                                <profiles>
-                                    <profile>integrationtests</profile>
-                                </profiles>
                                 <skipInvocation>${sting.packageintegrationtests.skipped}</skipInvocation>
                                 <properties>
+                                    <integrationtests.profile.enabled>true</integrationtests.profile.enabled>
+                                    <disable.gatk-framework.test.dependency>false</disable.gatk-framework.test.dependency>
                                     <sting.packageintegrationtests.skipped>${sting.packageintegrationtests.skipped}</sting.packageintegrationtests.skipped>
                                 </properties>
                             </configuration>
@@ -555,11 +556,10 @@
                                 <goals>
                                     <goal>verify</goal>
                                 </goals>
-                                <profiles>
-                                    <profile>integrationtests</profile>
-                                </profiles>
                                 <skipInvocation>${sting.packagepipelinetests.skipped}</skipInvocation>
                                 <properties>
+                                    <integrationtests.profile.enabled>true</integrationtests.profile.enabled>
+                                    <disable.gatk-framework.test.dependency>false</disable.gatk-framework.test.dependency>
                                     <sting.packagepipelinetests.skipped>${sting.packagepipelinetests.skipped}</sting.packagepipelinetests.skipped>
                                 </properties>
                             </configuration>
@@ -574,11 +574,10 @@
                                 <goals>
                                     <goal>verify</goal>
                                 </goals>
-                                <profiles>
-                                    <profile>integrationtests</profile>
-                                </profiles>
                                 <skipInvocation>${sting.packagelargescaletests.skipped}</skipInvocation>
                                 <properties>
+                                    <integrationtests.profile.enabled>true</integrationtests.profile.enabled>
+                                    <disable.gatk-framework.test.dependency>false</disable.gatk-framework.test.dependency>
                                     <sting.packagelargescaletests.skipped>${sting.packagelargescaletests.skipped}</sting.packagelargescaletests.skipped>
                                 </properties>
                             </configuration>
diff --git a/public/external-example/pom.xml b/public/external-example/pom.xml
index 37739bfda..5aba7ab26 100644
--- a/public/external-example/pom.xml
+++ b/public/external-example/pom.xml
@@ -211,11 +211,8 @@
                         <configuration>
                             <skip>false</skip>
                             <includes>
-                                <include>**/*Test.class</include>
+                                <include>**/*UnitTest.class</include>
                             </includes>
-                            <excludes>
-                                <exclude>**/*IntegrationTest.class</exclude>
-                            </excludes>
                         </configuration>
                     </execution>
                 </executions>
diff --git a/public/gatk-framework/pom.xml b/public/gatk-framework/pom.xml
index f4443a2f6..1443c7037 100644
--- a/public/gatk-framework/pom.xml
+++ b/public/gatk-framework/pom.xml
@@ -88,21 +88,55 @@
                     </execution>
                 </executions>
             </plugin>
+            <!--
+            TODO: disable.gatk-framework.test.dependency is required because the the same test classes
+            TODO: cannot be in both the gatk-framework test artifact AND the test classes directory.
+
+            TODO: Otherwise maven throws an [ERROR] java.lang.RuntimeException: Duplicate test set
+
+            TODO: This is due to AbstractSurefireMojo.scanForTestClasses() adding both scanDirectories() and scanDependencies().
+            TODO: Pain to debug as exception is three maven invoker levels deep, outer maven, maven-inovker-plugin, then forked tests!
+
+            TODO: Move BaseTest and other non UnitTests into the sting-utils test jar/artifact.
+
+            TODO: Then that dependency won't have test classes that will also run from the gatk-framework
+            TODO: test classes directory during the invoked package test.
+            -->
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-invoker-plugin</artifactId>
                 <executions>
                     <execution>
                         <id>package-unittests</id>
+                        <configuration>
+                            <properties>
+                                <disable.gatk-framework.test.dependency>true</disable.gatk-framework.test.dependency>
+                            </properties>
+                        </configuration>
                     </execution>
                     <execution>
                         <id>package-integrationtests</id>
+                        <configuration>
+                            <properties>
+                                <disable.gatk-framework.test.dependency>true</disable.gatk-framework.test.dependency>
+                            </properties>
+                        </configuration>
                     </execution>
                     <execution>
                         <id>package-largescaletests</id>
+                        <configuration>
+                            <properties>
+                                <disable.gatk-framework.test.dependency>true</disable.gatk-framework.test.dependency>
+                            </properties>
+                        </configuration>
                     </execution>
                     <execution>
                         <id>package-pipelinetests</id>
+                        <configuration>
+                            <properties>
+                                <disable.gatk-framework.test.dependency>true</disable.gatk-framework.test.dependency>
+                            </properties>
+                        </configuration>
                     </execution>
                 </executions>
             </plugin>
diff --git a/public/package-tests/pom.xml b/public/package-tests/pom.xml
index 3d1bc1136..3483f4da3 100644
--- a/public/package-tests/pom.xml
+++ b/public/package-tests/pom.xml
@@ -2,6 +2,10 @@
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
     <modelVersion>4.0.0</modelVersion>
 
+    <!--
+    Run during package testing by the maven invoker plugin in the sting-aggregator /pom.xml
+    -->
+
     <parent>
         <groupId>org.broadinstitute.sting</groupId>
         <artifactId>sting-root</artifactId>
@@ -15,6 +19,10 @@
 
     <properties>
         <sting.basedir>${project.basedir}/../..</sting.basedir>
+        <sting.packageunittests.skipped>true</sting.packageunittests.skipped>
+        <sting.packageintegrationtests.skipped>true</sting.packageintegrationtests.skipped>
+        <sting.packagepipelinetests.skipped>true</sting.packagepipelinetests.skipped>
+        <sting.packagelargescaletests.skipped>true</sting.packagelargescaletests.skipped>
     </properties>
 
     <!-- Dependency configuration (versions, etc.) -->
@@ -39,14 +47,6 @@
             <artifactId>cofoja</artifactId>
         </dependency>
 
-        <dependency>
-            <groupId>${project.groupId}</groupId>
-            <artifactId>gatk-framework</artifactId>
-            <version>${project.version}</version>
-            <type>test-jar</type>
-            <scope>test</scope>
-        </dependency>
-
         <dependency>
             <groupId>org.testng</groupId>
             <artifactId>testng</artifactId>
@@ -65,8 +65,36 @@
     </build>
 
     <profiles>
+        <!--
+        TODO: see description of disable.gatk-framework.test.dependency in gatk-framework/pom.xml
+        -->
+        <profile>
+            <id>include-gatk-framework-tests</id>
+            <activation>
+                <property>
+                    <name>disable.gatk-framework.test.dependency</name>
+                    <value>false</value>
+                </property>
+            </activation>
+            <dependencies>
+                <dependency>
+                    <groupId>${project.groupId}</groupId>
+                    <artifactId>gatk-framework</artifactId>
+                    <version>${project.version}</version>
+                    <type>test-jar</type>
+                    <scope>test</scope>
+                </dependency>
+            </dependencies>
+        </profile>
         <profile>
             <id>unittests</id>
+            <activation>
+                <activeByDefault>false</activeByDefault>
+                <property>
+                    <name>unittests.profile.enabled</name>
+                    <value>true</value>
+                </property>
+            </activation>
             <build>
                 <!-- Plugin configuration -->
                 <plugins>
@@ -101,6 +129,13 @@
 
         <profile>
             <id>integrationtests</id>
+            <activation>
+                <activeByDefault>false</activeByDefault>
+                <property>
+                    <name>integrationtests.profile.enabled</name>
+                    <value>true</value>
+                </property>
+            </activation>
             <build>
                 <!-- Plugin configuration -->
                 <plugins>
diff --git a/public/sting-root/pom.xml b/public/sting-root/pom.xml
index 1309175bb..83e9b21aa 100644
--- a/public/sting-root/pom.xml
+++ b/public/sting-root/pom.xml
@@ -389,13 +389,8 @@
                                 <!-- TODO: Re-enable unit tests by default? -->
                                 <skip>${sting.unittests.skipped}</skip>
                                 <includes>
-                                    <include>**/*Test.class</include>
+                                    <include>**/*UnitTest.class</include>
                                 </includes>
-                                <excludes>
-                                    <exclude>**/*IntegrationTest.class</exclude>
-                                    <exclude>**/*PipelineTest.class</exclude>
-                                    <exclude>**/*LargeScaleTest.class</exclude>
-                                </excludes>
                             </configuration>
                         </execution>
                     </executions>