Gradle now propagates its system props to tests.

* There are a couple of exceptions; see systemPropertiesBlackList in testing.gradle. * Removed Dap4Servelet from TDS's web.xml. The class no longer exists. * Deleted params.gradle. Propagation of system properties is now handled in testing.gradle. * gradle.properties no longer includes 'unidata.testdata.path' or 'tds.content.root.path'. Those are now system props, defined elsewhere. * Simplified the examination of the above 2 properties. We only check that they exist and denote a directory.
Unidata · May 20, 2015 · 8dd2b1f · 8dd2b1f
1 parent 8d10b0c
commit 8dd2b1f
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 200 deletions.
diff --git a/dap4/d4tests/build.gradle b/dap4/d4tests/build.gradle
@@ -21,6 +21,8 @@ dependencies {
 }
 
 test {
+    systemProperties['testargs'] = System.getProperty("testargs", "")
+
     include 'dap4/test/TestParserDMR.class'
     include 'dap4/test/TestParserCE.class'
     include 'dap4/test/TestNc4Iosp.class'

diff --git a/gradle.properties b/gradle.properties
@@ -3,6 +3,3 @@ org.gradle.daemon=false
 
 # This is for debugging Gradle itself. Best run with "--no-daemon".
 org.gradle.debug=false
-
-unidata.testdata.path=/share/testdata
-tds.content.root.path=/share/testdata/cdmUnitTest/it/content
diff --git a/gradle/params.gradle b/gradle/params.gradle
diff --git a/gradle/testing.gradle b/gradle/testing.gradle
@@ -18,24 +18,36 @@ ext {
 
 
     contentRootKey = 'tds.content.root.path'
-    isContentRootAvailable = isPropertyValidDirectory(contentRootKey)
+    isContentRootAvailable = isSystemPropertyAValidDirectory contentRootKey
 
     if (!isContentRootAvailable && !isJenkins) {        // Don't skip tests on Jenkins, except NotJenkins ones.
         logger.warn "Skipping all NeedsContentRoot tests."
     }
 
 
     testdataDirKey = 'unidata.testdata.path'
-    isCdmUnitTestDirAvailable = isPropertyValidDirectory(testdataDirKey)
+    isCdmUnitTestDirAvailable = isSystemPropertyAValidDirectory testdataDirKey
 
     if (!isCdmUnitTestDirAvailable && !isJenkins) {     // Don't skip tests on Jenkins, except NotJenkins ones.
         logger.warn "Skipping all NeedsCdmUnitTest tests."
     }
 
 
-    timesPrinted = 0
+    // Do not propagate these properties to the Gradle test executors.
+    systemPropertiesBlackList = [
+        // Propagating this causes "java.lang.ClassNotFoundException: org.jacoco.agent.rt.internal_932a715.PreMain"
+        // See https://discuss.gradle.org/t/jacoco-related-failure-in-multiproject-build/6216
+        'user.dir',
+        // Gradle is also using JNA and it sets this property when it runs. However, it is an old, incompatible version
+        // (3.2.7). We'll fail to load NetCDF-C if we try to do it using JNA v3.
+        // What we want is for JNA to look in the JNA jar file for the lib, which it'll do if this prop is *undefined*.
+        // See http://twall.github.io/jna/4.1.0/overview-summary.html#loading
+        'jna.boot.library.path'
+    ]
 }
 
+import java.nio.file.*
+
 /**
  * Returns {@code true} if the given system property is defined and denotes an existing directory. Otherwise,
  * {@code false} is returned and the property is set to {@code "$buildDir/NO/$sysPropKey/FOUND/"}.
@@ -51,7 +63,7 @@ boolean isSystemPropertyAValidDirectory(String sysPropKey) {
         if (Files.isDirectory(Paths.get(sysPropVal))) {
             return true
         } else {
-            logger.warn "-D$sysPropKey=\"$sysPropVal\"; value is not a directory."
+            logger.warn "$sysPropKey=\"$sysPropVal\"; system property is not a directory."
         }
     } else {
         logger.warn "\"$sysPropKey\" system property not defined."
@@ -67,79 +79,16 @@ boolean isSystemPropertyAValidDirectory(String sysPropKey) {
     // that property, the responsible place to create this log directory is under the project build directory.
     String defaultSysPropVal = Paths.get(buildDir.path, "NO", sysPropKey, "FOUND").toAbsolutePath().toString()
 
-    logger.info "Setting default property value: -D$propKey=\"$defaultPropVal\""
+    logger.info "Setting default system property: $sysPropKey=\"$defaultSysPropVal\""
     System.properties[sysPropKey] = defaultSysPropVal
     return false
 }
 
-/**
- * Returns {@code true} if the given Project property exists and denotes an existing directory in the file system.
- * Otherwise, {@code false} is returned and the property is set to {@code "/NO/$propKey/FOUND/"}.
- *
- * @param propKey the name of a property set in this Project. It'll likely have been loaded from gradle.properties.
- * @return {@code true} if the given Project property exists and denotes an existing directory in the file system.
- */
-boolean isPropertyValidDirectory(String propKey) {
-    def defaultPropVal = Paths.get(buildDir.path, "NO", propKey, "FOUND").toAbsolutePath().toString()
-
-    if (!hasProperty(propKey)) {
-        logger.warn "\"$propKey\" property not defined."
-        rootProject.ext[propKey] = defaultPropVal            // New property must be created in the ext namespace.
-        return false
-    } else {
-        String propVal = rootProject.property(propKey) as String;
-        Path dirPath = Paths.get propVal
-
-        if (!dirPath.isAbsolute()) {
-            logger.warn "$propKey=\"$propVal\"; value is not an absolute path in this file system."
-        } else if (Files.notExists(dirPath)) {
-            logger.warn "$propKey=\"$propVal\"; value is not an existing file."
-        } else if (!Files.isDirectory(dirPath)) {
-            logger.warn "$propKey=\"$propVal\"; value is not a directory."
-        } else {
-            return true
-        }
-
-        logger.info "Setting default property value: $propKey=\"$defaultPropVal\""
-        rootProject.setProperty propKey, defaultPropVal  // Property exists on Project; change its value.
-        return false
-    }
-}
-
-///**
-// * Returns {@code true} if the given Project property exists and denotes an existing directory in the file system.
-// * Otherwise, {@code false} is returned and the property is set to {@code "/NO/$propKey/FOUND/"}.
-// *
-// * @param propKey the name of a property set in this Project. It'll likely have been loaded from gradle.properties.
-// * @return {@code true} if the given Project property exists and denotes an existing directory in the file system.
-// */
-//boolean isPropertyValidDirectory(String propKey) {
-//    if (!hasProperty(propKey)) {
-//        logger.warn "\'$propKey\' property not defined."
-//        ext[propKey] = "/NO/$propKey/FOUND/"            // New property must be created in the ext namespace.
-//        return false
-//    } else {
-//        String propVal = property(propKey) as String;
-//
-//        if (!new File(propVal).isDirectory()) {
-//            logger.warn "\'$propVal\' directory not found."
-//            setProperty propKey, "/NO/$propKey/FOUND/"  // Property exists on Project; change its value.
-//            return false
-//        } else {
-//            return true
-//        }
-//    }
-//}
-
 
 configure(javaProjects) {
-    apply plugin: 'java'  // For testCompile configuration.
+    apply plugin: 'java'
     apply from: "$rootDir/gradle/dependencies.gradle"
 
-test {
-    systemProperties['testargs'] =
-            System.getProperty("testargs", "")
-}
     dependencies {
         testCompile libraries["junit"]
 
@@ -149,21 +98,35 @@ test {
         }
     }
 
-
     tasks.withType(Test).all {
-        if (timesPrinted == 0) {
-            println "===System Properties ($path)==="
-            System.properties.each { key, value ->
-                println "\t$key = $value"
+        // Propagates system properties set on the Gradle process to the test executors.
+        System.properties.each { key, value ->
+            if (!systemPropertiesBlackList.contains(key) &&
+                    !systemProperties.containsKey(key)) {  // Don't overrwrite any existing entries.
+                systemProperties[key] = value
             }
+        }
 
-            timesPrinted++
+        if (isTravis) {
+            ignoreFailures = false  // On Travis, abort the build if any tests fail.
+        } else {
+            ignoreFailures = true   // In every other environment, don't let test failures abort the build.
         }
 
-        // Propagates all system properties set on the Gradle process to the test executors.
-        systemProperties System.properties
-    }
+        useJUnit {
+            if (isJenkins) {
+                excludeCategories 'ucar.unidata.test.util.NotJenkins'
+            }
+
+            if (!isContentRootAvailable && !isJenkins) {     // Don't skip tests on Jenkins, except NotJenkins ones.
+                excludeCategories 'ucar.unidata.test.util.NeedsContentRoot'
+            }
 
+            if (!isCdmUnitTestDirAvailable && !isJenkins) {  // Don't skip tests on Jenkins, except NotJenkins ones.
+                excludeCategories 'ucar.unidata.test.util.NeedsCdmUnitTest'
+            }
+        }
+    }
 
     Project subproject = delegate as Project
 
@@ -189,97 +152,11 @@ boolean testRuntimeHasDepNamed(Project project, String depName) {
     }
 }
 
-// Process test exclusions.
-gradle.taskGraph.whenReady { TaskExecutionGraph taskGraph ->
-    Collection<Task> testTasks = taskGraph.allTasks.findAll { it instanceof Test }
-    if (!testTasks) {
-        return  // We're not running any tests.
-    }
-
-    println "===System Properties==="
-    System.properties.each { key, value ->
-        println "\t$key = $value"
-    }
-
-    // These appear to be the only environment variables that Jenkins defines: http://goo.gl/iCh08k
-    // Is there a better way to detect Jenkins?
-    boolean isJenkins = System.env['JENKINS_URL']  // We only care if prop is defined, not its actual value.
-
-    boolean isTravis = System.env['TRAVIS']
-
-    // We only want to write these log messages once, which is why we don't have them in the "testTasks*.configure{}"
-    // block below.
-    if (isJenkins) {
-        logger.warn "Skipping all NotJenkins tests: detected that we're running in the Jenkins environment."
-    }
-
-    if (isTravis) {
-        logger.warn "Skipping all NotTravis tests: detected that we're running in the Travis environment."
-    }
-
-    String contentRootKey = "tds.content.root.path"
-    boolean isContentRootAvailable = isPropertyValidDirectory(contentRootKey)
-    if (!isContentRootAvailable && !isJenkins) {        // Don't skip tests on Jenkins, except NotJenkins ones.
-        logger.warn "Skipping all NeedsContentRoot tests."
-    }
-
-    String testdataDirKey = "unidata.testdata.path"
-    boolean isCdmUnitTestDirAvailable = isPropertyValidDirectory(testdataDirKey)
-    if (!isCdmUnitTestDirAvailable && !isJenkins) {     // Don't skip tests on Jenkins, except NotJenkins ones.
-        logger.warn "Skipping all NeedsCdmUnitTest tests."
-    }
-
-    // FIXME: Should I be using org.gradle.api.DomainObjectCollection.all(Closure) here instead?
-    testTasks*.configure {
-        useJUnit {
-            // Propagate to Gradle Test Executor.
-            String jnaKey = 'jna.library.path'
-            String jnaVal = System.getProperty(jnaKey)
-            if (jnaVal) {
-                systemProperty jnaKey, jnaVal
-            }
-
-            if (isJenkins) {
-                excludeCategories 'ucar.unidata.test.util.NotJenkins'
-            }
-
-            if (isTravis) {
-                ignoreFailures false  // On Travis, abort the build if any tests fail.
-            } else {
-                ignoreFailures true  // In every other environment, don't let test failures abort the build.
-            }
-
-            systemProperty contentRootKey, rootProject.property(contentRootKey)  // Propagate to Gradle Test Executor.
-
-            if (!isContentRootAvailable && !isJenkins) {        // Don't skip tests on Jenkins, except NotJenkins ones.
-                excludeCategories 'ucar.unidata.test.util.NeedsContentRoot'
-
-                if (path == ":it:integrationTest") {
-                    logger.warn "Skipping all integration tests (task \'$path\'): " +
-                            "\'$contentRootKey\' property not defined."
-
-                    // We can't stand up a functioning TDS test instance without a valid content root,
-                    // so disable the integration test task altogether.
-                    enabled = false
-                }
-            }
-
-            systemProperty testdataDirKey, rootProject.property(testdataDirKey)  // Propagate to Gradle Test Executor.
-
-            if (!isCdmUnitTestDirAvailable && !isJenkins) {     // Don't skip tests on Jenkins, except NotJenkins ones.
-                excludeCategories 'ucar.unidata.test.util.NeedsCdmUnitTest'
-            }
-        }
-    }
-
-    // Add the content root and testdata dir to :it's gretty extension as system properties.
-    // This is how we propagate these properties to the embedded Tomcat that's started for :it:integrationTest.
-    project(":it").gretty.systemProperty contentRootKey, rootProject.property(contentRootKey)
-    project(':it').gretty.systemProperty testdataDirKey, rootProject.property(testdataDirKey)
-}
-
 ///////////////////////////////////////////////// Root /////////////////////////////////////////////////
 
+// FIXME: I'm not sure if either of these consider :it:integrationTest. That would make sense, because it hasn't been
+// evaluated yet. How best to fix?
+
 task testAll(group: 'Build') {
     description = 'Runs all subproject Test tasks'
     dependsOn subprojects*.tasks*.withType(Test)

diff --git a/it/build.gradle b/it/build.gradle
@@ -59,6 +59,16 @@ gretty {
 
 // Gretty will start the webapp before this task and shut it down afterward.
 task integrationTest(type: Test) {
+    // isContentRootAvailable, isJenkins, and contentRootKey are defined on the root project in testing.gradle.
+    if (!isContentRootAvailable && !isJenkins) {  // Don't skip tests on Jenkins, except NotJenkins ones.
+        logger.warn "Skipping all integration tests (task \'$path\'): " +
+                "\'$contentRootKey\' property not defined."
+
+        // We can't stand up a functioning TDS test instance without a valid content root,
+        // so disable the integration test task altogether.
+        enabled = false
+    }
+
     // From TDS's pom.xml. I guess these are excluded because they're pretty crufty?
     exclude 'thredds/tds/**'
 
@@ -74,6 +84,11 @@ task integrationTest(type: Test) {
     }
 }
 
+// Propagate all of integrationTest's system properties (which were set in testing.gradle) to the embedded Tomcat
+// instance. org.akhikhl.gretty.ServerConfig.systemProperties is null by default.
+// So, we can be sure that we're not trampling over any existing properties by doing this.
+gretty.systemProperties integrationTest.systemProperties
+
 task integrationTestReport(type: JacocoReport, dependsOn: "integrationTest") {
     // executionData must be closure, because appBeforeIntegrationTest is not defined yet.
     // We must pass jacoco.destinationFile, not appBeforeIntegrationTest itself,

diff --git a/tds/src/main/webapp/WEB-INF/web.xml b/tds/src/main/webapp/WEB-INF/web.xml
@@ -236,13 +236,6 @@
     <servlet-class>thredds.server.opendap.OpendapServlet</servlet-class>
     <load-on-startup>2</load-on-startup>
   </servlet>
-  <!-- NetCDF/Dap4 server -->
-  <servlet>
-    <display-name>Dap4 Server</display-name>
-    <servlet-name>dap4</servlet-name>
-    <servlet-class>thredds.server.dap4.Dap4Servlet</servlet-class>
-    <load-on-startup>3</load-on-startup>
-  </servlet>
 
   <!-- HTTP File server -->
   <!-- servlet>