From 8b8f12ab1671ad6d7ef54c971fe1e829624ec6c4 Mon Sep 17 00:00:00 2001 From: Arunachalam Thirupathi Date: Fri, 30 May 2014 11:23:45 -0700 Subject: [PATCH] Fix the junit tests, bump sleep for bind issues 1) Merged the changes from Szczepan for fixing the Junit tests. Previously JunitAll ran the integration tests and ran over 4 hours. 2) Szczepan removed redundant sourceSets, moved common configurations to test block, created reports 3) Created aggregated reports for junitAll verified the coverage was similar to before. 4) Noticed few bind exceptions on 2 ms sleep, so bumped them to 5 ms. --- .gitignore | 2 +- build.gradle | 168 ++++++++------------- .../rebalance/AbstractZonedRebalanceTest.java | 2 +- .../ZonedRebalanceNonContiguousZonesTest.java | 62 ++++---- 4 files changed, 96 insertions(+), 138 deletions(-) diff --git a/.gitignore b/.gitignore index 56f131add..b871520fb 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ -config/single_node_cluster/config/STORES +config/*/config/STORES classes dist *~ diff --git a/build.gradle b/build.gradle index 58013964d..8f2a11224 100644 --- a/build.gradle +++ b/build.gradle @@ -2,6 +2,7 @@ import java.util.jar.JarEntry; apply plugin: 'java' apply plugin: 'scala' +apply plugin: 'idea' def String getProjectProperty(String propertyName) { String propertyValue = "null" @@ -23,7 +24,8 @@ def libDir = getProjectProperty('lib.dir') def resourcesDir = getProjectProperty('resources.dir') def javaDocDir = getProjectProperty('javadoc.dir') -def testClassesDir = getProjectProperty('testclasses.dir') +def voldTestClassesDir = getProjectProperty('testclasses.dir') + def commonTestSrcDir = getProjectProperty('commontestsrc.dir') def unitTestSrcDir = getProjectProperty('unittestsrc.dir') def intTestSrcDir = getProjectProperty('inttestsrc.dir') @@ -74,11 +76,12 @@ sourceSets { } resources { srcDirs = [javaDir] - include '**/*.xsd' + include '**/*.xsd', 'log4j.properties' } output.classesDir = classesDir + output.resourcesDir = resourcesDir } - buildTest { + test { java { srcDirs = [ commonTestSrcDir , @@ -87,61 +90,27 @@ sourceSets { longTestSrcDir ] } - output.classesDir = testClassesDir - compileClasspath += sourceSets.main.runtimeClasspath - } - test { - //There is some gradle weird behavior going on here. If I dont have standard tests - // and only other tests like junitLong gradle completes without running any tests - // and says junit upto date. So including this block and controlling in the task - // block to run only client and server tests which takes 3 minutes. - - java { - srcDirs = [unitTestSrcDir ] - } - resources { - srcDir resourcesDir - } - compileClasspath += sourceSets.main.runtimeClasspath + sourceSets.buildTest.runtimeClasspath + output.classesDir = voldTestClassesDir } contrib { - java { srcDirs = [contribRootDir]} - resources { - srcDir resourcesDir - } + java { srcDirs = [contribRootDir] } compileClasspath += sourceSets.main.runtimeClasspath output.classesDir = contribClassesDir } - junitLong { - java { - srcDirs = [longTestSrcDir ] - } - resources { - srcDir resourcesDir - } - compileClasspath += sourceSets.main.runtimeClasspath + sourceSets.buildTest.runtimeClasspath - } } -compileJava.dependsOn clean - -task compileJava.doLast { +compileJava.doLast { project.copy { from (javaDir) { exclude '**/*.java','**/*.html','**/*.scala', '**/log4j.properties' } into classesDir } - - project.copy { - from (javaDir) { include 'log4j.properties' } - into resourcesDir - } } -compileBuildTestJava.doLast { +compileTestJava.doLast { project.copy { from (commonTestSrcDir) { exclude '**/*.java','**/*.html' } from (unitTestSrcDir) { exclude '**/*.java','**/*.html' } - into testClassesDir + into voldTestClassesDir } } @@ -152,23 +121,19 @@ compileContribJava.doLast { } } - dependencies { compile fileTree(dir: libDir, includes: ['**/*.jar']) - buildTestCompile sourceSets.main.output - contribCompile sourceSets.main.output - contribCompile sourceSets.buildTest.output - testCompile sourceSets.buildTest.output + contribCompile sourceSets.test.output + contribCompile fileTree(dir: contribRootDir, includes: ['**/*.jar']) - buildTestCompile 'junit:junit:4.6' + testCompile 'junit:junit:4.6' } task testJar(type: Jar) { - dependsOn sourceSets.buildTest.output baseName = projectName + "-test" - from sourceSets.buildTest.output + from sourceSets.test.output destinationDir = project.file(distDir) } @@ -191,7 +156,7 @@ task contribJar(type:Jar) { destinationDir = project.file(distDir) } -task sourcesJar(type: Jar, dependsOn: classes) { +task srcJar(type: Jar, dependsOn: classes) { classifier = 'src' from sourceSets.main.java.srcDirs destinationDir = project.file(distDir) @@ -201,7 +166,7 @@ artifacts { archives voldJar archives testJar archives contribJar - archives sourcesJar + archives srcJar } clean { @@ -250,7 +215,7 @@ task tar (type: Tar) { destinationDir = project.file(distDir) } -jar.dependsOn contribJar,sourcesJar +jar.dependsOn contribJar,srcJar compileContribJava.dependsOn voldJar copySources.dependsOn jar @@ -267,6 +232,12 @@ tasks.withType(Test) { maxHeapSize = "2g" forkEvery = 1 + + // If ignoreFailures is not set, then merged reports will not be generated + // Gradle aborts further tasks on test failure. so if you run junitAll + // which runs 3 tests, reports task will never be run on failure cases. + ignoreFailures = true + useJUnit() testLogging { @@ -278,91 +249,78 @@ tasks.withType(Test) { logger.lifecycle("testFinished: $test, result: $result.resultType") } + doFirst { + def classesSize = candidateClassFiles.files.size() + logger.lifecycle("{} starts executing {} test classes {}", + path, classesSize, classesSize > 0? "(" + candidateClassFiles.files*.name[0] + ", ...)" : "") + } + //all standard error messages from tests will get routed to 'DEBUG' level messages. //logging.captureStandardError(LogLevel.DEBUG) //all standard output messages from tests will get routed to 'DEBUG' level messages. //logging.captureStandardOutput(LogLevel.DEBUG) + + //Set reasonable defaults for reports location + reports.html.destination = file("$project.buildDir/reports/$name") + reports.junitXml.destination = file("$project.buildDir/$name-results") + + //Set reasonable defaults classpath and classes dir. They can be reconfigured in an individual task. + it.testClassesDir = sourceSets.test.output.classesDir + classpath = sourceSets.test.runtimeClasspath } -task junit(type: Test) { - dependsOn buildTestRuntime - description = "Runs acceptance tests" +task junit(dependsOn: test) - include '**/*Test.*' - exclude '**/Abstract*' - testClassesDir = sourceSets.test.output.classesDir - classpath += sourceSets.test.runtimeClasspath +Collection testClassesFrom(String dir, String include = '**/*Test.*') { + //take all *Test.java files found in given dir, make the path relative and replace .java with .class + fileTree(dir: dir, includes: [include]).collect { it.absolutePath.replaceAll(file(dir).absolutePath + "/", "").replaceAll(".java\$", ".class")} +} +test { + description = "Runs acceptance tests" + include testClassesFrom(unitTestSrcDir) } task junitLong(type: Test) { - dependsOn buildTestRuntime description = "Runs long junit tests" - - include '**/*Test.*' - testClassesDir = sourceSets.junitLong.output.classesDir - classpath += sourceSets.junitLong.runtimeClasspath - + include testClassesFrom(longTestSrcDir) } task junitRebalance(type: Test) { - dependsOn buildTestRuntime - description = "Runs acceptance tests" - - include '**/*Rebalance*Test.*' - exclude '**/Abstract*' - testClassesDir = sourceSets.test.output.classesDir - classpath += sourceSets.test.runtimeClasspath + include testClassesFrom(unitTestSrcDir, '**/*Rebalance*Test.java') } task junitRebalanceLong(type: Test) { - dependsOn buildTestRuntime - description = "Runs acceptance tests" - - include '**/*Rebalance*Test.*' - testClassesDir = sourceSets.junitLong.output.classesDir - classpath += sourceSets.junitLong.runtimeClasspath + include testClassesFrom(longTestSrcDir, '**/*Rebalance*Test.java') } task contribJunit(type: Test) { - dependsOn buildTestRuntime description = "Run contrib junit tests except EC2 and Krati tests." + it.testClassesDir = file(contribClassesDir) - include '**/*Test.class' exclude '**/*PerformanceTest.class' exclude '**/*RemoteTest.class' exclude '**/Ec2*Test.class' exclude '**/Krati*Test.class' exclude '**/HadoopStoreBuilder*Test.class' - testClassesDir = sourceSets.contrib.output.classesDir - classpath += sourceSets.contrib.runtimeClasspath -} -task junitAll(type: Test) { - dependsOn junitLong - dependsOn contribJunit + classpath += sourceSets.contrib.runtimeClasspath + sourceSets.contrib.output } -test { - // Gradle is really weird. if I dont have this block, all the other tests does - // not run and says UP-T0-DATE without running anything. If I keep this block - // and says exclude **/* "gradle junit" has 349 test failures due to resources. - - // So keeping this block to let it pass. Will follow up with our gradle expert - // to understand what is going on here. You can avoid running tests all together - // by running "gradle jar" which avoids the testing phase completely. - - // If this block is defined above the other tests, the tests could not find the - // resources. - dependsOn buildTestRuntime - description = "Runs acceptance tests" +task junitAll(type: TestReport) { + reportOn test, junitLong, contribJunit + destinationDir = file("$project.buildDir/reports/$name") +} - include '**/*Server*Test.*' - include '**/*Client*Test.*' - exclude '**/Abstract*' +task aggregatedJunit(type: TestReport) { +destinationDir = file("$project.buildDir/reports/$name") +} - testClassesDir = sourceSets.test.output.classesDir - classpath += sourceSets.test.runtimeClasspath +tasks.withType(Test) { + finalizedBy aggregatedJunit + doLast { + aggregatedJunit.reportOn it.binResultsDir + } } diff --git a/test/unit/voldemort/client/rebalance/AbstractZonedRebalanceTest.java b/test/unit/voldemort/client/rebalance/AbstractZonedRebalanceTest.java index cee2f6a71..e0a7d2db5 100644 --- a/test/unit/voldemort/client/rebalance/AbstractZonedRebalanceTest.java +++ b/test/unit/voldemort/client/rebalance/AbstractZonedRebalanceTest.java @@ -329,7 +329,7 @@ public abstract class AbstractZonedRebalanceTest extends AbstractRebalanceTest { // Hacky work around of TOCTOU bind Exception issues. Each test that // invokes this method brings servers up & down on the same ports. The // OS seems to need a rest between subsequent tests... - Thread.sleep(TimeUnit.SECONDS.toMillis(2)); + Thread.sleep(TimeUnit.SECONDS.toMillis(5)); Cluster interimCluster = RebalanceUtils.getInterimCluster(zzCurrent, zzShuffle); diff --git a/test/unit/voldemort/client/rebalance/ZonedRebalanceNonContiguousZonesTest.java b/test/unit/voldemort/client/rebalance/ZonedRebalanceNonContiguousZonesTest.java index f92b7c14c..0932865ca 100644 --- a/test/unit/voldemort/client/rebalance/ZonedRebalanceNonContiguousZonesTest.java +++ b/test/unit/voldemort/client/rebalance/ZonedRebalanceNonContiguousZonesTest.java @@ -1,12 +1,12 @@ /* * Copyright 2008-2013 LinkedIn, Inc - * + * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the @@ -249,7 +249,7 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest * Scripts the execution of a specific type of zoned rebalance test: sets up * cluster based on cCluster plus any new nodes/zones in fCluster, * rebalances to fCluster, verifies rebalance was correct. - * + * * @param testTag For pretty printing * @param cCluster current cluster * @param fCluster final cluster @@ -268,9 +268,9 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest List cStoreDefs, List fStoreDefs) throws Exception { logger.info("Starting " + testTag); - // Hacky work around of TOCTOU bind Exception issues. Each test that invokes this method brings + // Hacky work around of TOCTOU bind Exception issues. Each test that invokes this method brings // servers up & down on the same ports. The OS seems to need a rest between subsequent tests... - Thread.sleep(TimeUnit.SECONDS.toMillis(2)); + Thread.sleep(TimeUnit.SECONDS.toMillis(5)); try { Cluster interimCluster = RebalanceUtils.getInterimCluster(cCluster, fCluster); List serverList = new ArrayList(interimCluster.getNodeIds()); @@ -317,9 +317,9 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest public void testShuffleZ1Z3AndShuffleAgain() throws Exception { logger.info("Starting testShuffleZZAndShuffleAgain"); - // Hacky work around of TOCTOU bind Exception issues. Each test that invokes this method brings servers + // Hacky work around of TOCTOU bind Exception issues. Each test that invokes this method brings servers // up & down on the same ports. The OS seems to need a rest between subsequent tests... - Thread.sleep(TimeUnit.SECONDS.toMillis(2)); + Thread.sleep(TimeUnit.SECONDS.toMillis(5)); Cluster interimCluster = RebalanceUtils.getInterimCluster(z1z3Current, z1z3Shuffle); @@ -418,7 +418,7 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest configProps.put("admin.max.threads", "5"); if(serial) { configProps.put("max.parallel.stores.rebalancing", String.valueOf(1)); - } + } currentCluster = startServers(currentCluster, storeDefFileWithReplication, serverList, configProps); String bootstrapUrl = getBootstrapUrl(currentCluster, 3); int maxParallel = 5; @@ -471,12 +471,12 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest /** * original server partition ownership - * + * * [s3 : p0,p3,p4,p5,p6,p7] [s4 : p1-p7] [s5 : p1,p2] [s9 : p0,p1,p2,p3,p6,p7] [s10 : p1-p7] [s11 : p4,p5] - * + * * final server partition ownership - * - * [s3 : p0,p2,p3,p4,p5,p6,p7] [s4 : p0,p1] [s5 : p1-p7] [s9 : p0.p1,p2,p3,p5,p6,p7] + * + * [s3 : p0,p2,p3,p4,p5,p6,p7] [s4 : p0,p1] [s5 : p1-p7] [s9 : p0.p1,p2,p3,p5,p6,p7] * [s10 : p0,p1,p2,p3,p4,p7] [s11 : p4,p5,p6] */ @@ -519,7 +519,7 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest ServerTestUtils.waitForAsyncOperationOnServer(serverMap.get(serverList.get(i)), "Repair", 5000); } - // confirm a primary movement in zone 1 : P6 : s4 -> S5. The zone 1 primary changes when + // confirm a primary movement in zone 1 : P6 : s4 -> S5. The zone 1 primary changes when // p6 moves cross zone check for existence of p6 in server 5, checkForKeyExistence(admin, 5, rwStoreDefWithReplication.getName(), p6KeySamples); @@ -530,7 +530,7 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest // check for its absernce in server 4 // also check that p1 is stable in server 4 [primary stability] checkForKeyExistence(admin, 4, rwStoreDefWithReplication.getName(), p1KeySamples); - + // check that p3 is stable in server 3 [Secondary stability] checkForKeyExistence(admin, 3, rwStoreDefWithReplication.getName(), p3KeySamples); @@ -712,29 +712,29 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest /** * Original partition map - * + * * [s3 : p0] [s4 : p1, p6] [s5 : p2] - * + * * [s9 : p3] [s10 : p4, p7] [s11 : p5] - * + * * final server partition ownership - * + * * [s3 : p0] [s4 : p1] [s5 : p2, p7] - * + * * [s9 : p3] [s10 : p4] [s11 : p5, p6] - * + * * Note that rwStoreDefFileWithReplication is a "2/1/1" store def. - * + * * Original server n-ary partition ownership - * + * * [s3 : p0, p3-7] [s4 : p0-p7] [s5 : p1-2] - * + * * [s9 : p0-3, p6-7] [s10 : p0-p7] [s11 : p4-5] - * + * * final server n-ary partition ownership - * + * * [s3 : p0, p2-7] [s4 : p0-1] [s5 : p1-p7] - * + * * [s9 : p0-3, p5-7] [s10 : p0-4, p7] [s11 : p4-6] */ List serverList = Arrays.asList(3, 4, 5, 9, 10, 11); @@ -749,7 +749,7 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest final List exceptions = Collections.synchronizedList(new ArrayList()); // Its is imperative that we test in a single shot since multiple batches would mean the proxy bridges - // being torn down and established multiple times and we cannot test against the source + // being torn down and established multiple times and we cannot test against the source // cluster topology then. getRebalanceKit uses batch size of infinite, so this should be fine. String bootstrapUrl = getBootstrapUrl(updatedCurrentCluster, 3); int maxParallel = 2; @@ -824,7 +824,7 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest null, factory, 3); - // Now perform some writes and determine the end state of the changed keys. + // Now perform some writes and determine the end state of the changed keys. // Initially, all data now with zero vector clock for(ByteArray movingKey: movingKeysList) { try { @@ -873,7 +873,7 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest executors.shutdown(); executors.awaitTermination(300, TimeUnit.SECONDS); - assertEquals("Client did not see all server transition into rebalancing state", + assertEquals("Client did not see all server transition into rebalancing state", rebalancingStarted.get(), true); assertEquals("Not enough time to begin proxy writing", proxyWritesDone.get(), true); checkEntriesPostRebalance(updatedCurrentCluster, finalCluster, @@ -907,7 +907,7 @@ public class ZonedRebalanceNonContiguousZonesTest extends AbstractRebalanceTest throw ae; } } - + private void populateData(Cluster cluster, StoreDefinition storeDef) throws Exception { // Create SocketStores for each Node first -- 2.11.4.GIT