From 3ab78543a737223be36421ff2cef7757fa4b1bb1 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Tue, 5 Mar 2013 01:27:36 -0500 Subject: [PATCH] Fix tests that were consistently or intermittently failing when run in parallel on the farm -Make MaxRuntimeIntegrationTest more lenient by assuming that startup overhead might be as long as 120 seconds on a very slow node, rather than the original assumption of 20 seconds -In TraverseActiveRegionsUnitTest, write temp bam file to the temp directory, not to the current working directory -SimpleTimerUnitTest: This test was internally inconsistent. It asserted that a particular operation should take no more than 10 milliseconds, and then asserted again that this same operation should take no more than 100 microseconds (= 0.1 millisecond). On a slow node it could take slightly longer than 100 microseconds, however. Changed the test to assert that the operation should require no more than 10000 microseconds (= 10 milliseconds) -change global default test timeout from 20 to 40 minutes (things just take longer on the farm!) -build.xml: allow runtestonly target to work with scala test classes --- build.xml | 1 + .../sting/TestNGTestTransformer.java | 4 +-- .../sting/gatk/MaxRuntimeIntegrationTest.java | 4 +-- .../TraverseActiveRegionsUnitTest.java | 29 ++++++++++--------- .../sting/utils/SimpleTimerUnitTest.java | 4 +-- 5 files changed, 22 insertions(+), 20 deletions(-) diff --git a/build.xml b/build.xml index 03f3232f2..fb5362b3e 100644 --- a/build.xml +++ b/build.xml @@ -1473,6 +1473,7 @@ + diff --git a/public/java/test/org/broadinstitute/sting/TestNGTestTransformer.java b/public/java/test/org/broadinstitute/sting/TestNGTestTransformer.java index 362d409cb..772c86563 100644 --- a/public/java/test/org/broadinstitute/sting/TestNGTestTransformer.java +++ b/public/java/test/org/broadinstitute/sting/TestNGTestTransformer.java @@ -35,7 +35,7 @@ import java.lang.reflect.Method; /** * Provide default @Test values for GATK testng tests. * - * Currently only sets the maximum runtime to 10 minutes, if it's not been specified. + * Currently only sets the maximum runtime to 40 minutes, if it's not been specified. * * See http://beust.com/weblog/2006/10/18/annotation-transformers-in-java/ * @@ -44,7 +44,7 @@ import java.lang.reflect.Method; * @version 0.1 */ public class TestNGTestTransformer implements IAnnotationTransformer { - public static final long DEFAULT_TIMEOUT = 1000 * 60 * 20; // 20 minutes max per test + public static final long DEFAULT_TIMEOUT = 1000 * 60 * 40; // 40 minutes max per test final static Logger logger = Logger.getLogger(TestNGTestTransformer.class); diff --git a/public/java/test/org/broadinstitute/sting/gatk/MaxRuntimeIntegrationTest.java b/public/java/test/org/broadinstitute/sting/gatk/MaxRuntimeIntegrationTest.java index 55f9e1f7d..25ee9ff09 100644 --- a/public/java/test/org/broadinstitute/sting/gatk/MaxRuntimeIntegrationTest.java +++ b/public/java/test/org/broadinstitute/sting/gatk/MaxRuntimeIntegrationTest.java @@ -39,7 +39,7 @@ import java.util.concurrent.TimeUnit; * */ public class MaxRuntimeIntegrationTest extends WalkerTest { - private static final long STARTUP_TIME = TimeUnit.NANOSECONDS.convert(20, TimeUnit.SECONDS); + private static final long STARTUP_TIME = TimeUnit.NANOSECONDS.convert(120, TimeUnit.SECONDS); private class MaxRuntimeTestProvider extends TestDataProvider { final long maxRuntime; @@ -68,7 +68,7 @@ public class MaxRuntimeIntegrationTest extends WalkerTest { // // Loop over errors to throw, make sure they are the errors we get back from the engine, regardless of NT type // - @Test(enabled = true, dataProvider = "MaxRuntimeProvider", timeOut = 60 * 1000) + @Test(enabled = true, dataProvider = "MaxRuntimeProvider", timeOut = 300 * 1000) public void testMaxRuntime(final MaxRuntimeTestProvider cfg) { WalkerTest.WalkerTestSpec spec = new WalkerTest.WalkerTestSpec( "-T PrintReads -R " + hg18Reference diff --git a/public/java/test/org/broadinstitute/sting/gatk/traversals/TraverseActiveRegionsUnitTest.java b/public/java/test/org/broadinstitute/sting/gatk/traversals/TraverseActiveRegionsUnitTest.java index a574932a7..0384260fa 100644 --- a/public/java/test/org/broadinstitute/sting/gatk/traversals/TraverseActiveRegionsUnitTest.java +++ b/public/java/test/org/broadinstitute/sting/gatk/traversals/TraverseActiveRegionsUnitTest.java @@ -58,6 +58,7 @@ import org.testng.annotations.Test; import java.io.File; import java.io.FileNotFoundException; +import java.io.IOException; import java.util.*; /** @@ -86,11 +87,10 @@ public class TraverseActiveRegionsUnitTest extends BaseTest { private List intervals; - private static final String testBAM = "TraverseActiveRegionsUnitTest.bam"; - private static final String testBAI = "TraverseActiveRegionsUnitTest.bai"; + private File testBAM; @BeforeClass - private void init() throws FileNotFoundException { + private void init() throws IOException { //reference = new CachingIndexedFastaSequenceFile(new File("/Users/depristo/Desktop/broadLocal/localData/human_g1k_v37.fasta")); // hg19Reference)); reference = new CachingIndexedFastaSequenceFile(new File(hg19Reference)); dictionary = reference.getSequenceDictionary(); @@ -133,17 +133,18 @@ public class TraverseActiveRegionsUnitTest extends BaseTest { createBAM(reads); } - private void createBAM(List reads) { - File outFile = new File(testBAM); - outFile.deleteOnExit(); - File indexFile = new File(testBAI); - indexFile.deleteOnExit(); + private void createBAM(List reads) throws IOException { + testBAM = File.createTempFile("TraverseActiveRegionsUnitTest", ".bam"); + testBAM.deleteOnExit(); - SAMFileWriter out = new SAMFileWriterFactory().setCreateIndex(true).makeBAMWriter(reads.get(0).getHeader(), true, outFile); + SAMFileWriter out = new SAMFileWriterFactory().setCreateIndex(true).makeBAMWriter(reads.get(0).getHeader(), true, testBAM); for (GATKSAMRecord read : ReadUtils.sortReadsByCoordinate(reads)) { out.addAlignment(read); } out.close(); + + new File(testBAM.getAbsolutePath().replace(".bam", ".bai")).deleteOnExit(); + new File(testBAM.getAbsolutePath() + ".bai").deleteOnExit(); } @Test(enabled = true && ! DEBUG, dataProvider = "TraversalEngineProvider") @@ -400,7 +401,7 @@ public class TraverseActiveRegionsUnitTest extends BaseTest { return getActiveRegions(t, walker, intervals, testBAM); } - private Map getActiveRegions(TraverseActiveRegions t, DummyActiveRegionWalker walker, List intervals, final String bam) { + private Map getActiveRegions(TraverseActiveRegions t, DummyActiveRegionWalker walker, List intervals, final File bam) { for (LocusShardDataProvider dataProvider : createDataProviders(t, walker, intervals, bam)) t.traverse(walker, dataProvider, 0); @@ -466,13 +467,13 @@ public class TraverseActiveRegionsUnitTest extends BaseTest { return record; } - private List createDataProviders(TraverseActiveRegions traverseActiveRegions, final Walker walker, List intervals, String bamFile) { + private List createDataProviders(TraverseActiveRegions traverseActiveRegions, final Walker walker, List intervals, File bamFile) { GenomeAnalysisEngine engine = new GenomeAnalysisEngine(); engine.setGenomeLocParser(genomeLocParser); traverseActiveRegions.initialize(engine, walker); Collection samFiles = new ArrayList(); - SAMReaderID readerID = new SAMReaderID(new File(bamFile), new Tags()); + SAMReaderID readerID = new SAMReaderID(bamFile, new Tags()); samFiles.add(readerID); SAMDataSource dataSource = new SAMDataSource(samFiles, new ThreadAllocation(), null, genomeLocParser, @@ -594,7 +595,7 @@ public class TraverseActiveRegionsUnitTest extends BaseTest { walker.setStates(readStates); final TraverseActiveRegions traversal = new TraverseActiveRegions(); - final Map activeRegionsMap = getActiveRegions(traversal, walker, intervals, bamBuilder.makeTemporarilyBAMFile().toString()); + final Map activeRegionsMap = getActiveRegions(traversal, walker, intervals, bamBuilder.makeTemporarilyBAMFile()); final Set alreadySeenReads = new HashSet(); // for use with the primary / non-primary for ( final ActiveRegion region : activeRegionsMap.values() ) { @@ -666,7 +667,7 @@ public class TraverseActiveRegionsUnitTest extends BaseTest { final DummyActiveRegionWalker walker = new DummyActiveRegionWalker(activeRegions, false); final TraverseActiveRegions traversal = new TraverseActiveRegions(); - final Map activeRegionsMap = getActiveRegions(traversal, walker, intervals, bamBuilder.makeTemporarilyBAMFile().toString()); + final Map activeRegionsMap = getActiveRegions(traversal, walker, intervals, bamBuilder.makeTemporarilyBAMFile()); final ActiveRegion region = activeRegionsMap.values().iterator().next(); int nReadsExpectedInRegion = 0; diff --git a/public/java/test/org/broadinstitute/sting/utils/SimpleTimerUnitTest.java b/public/java/test/org/broadinstitute/sting/utils/SimpleTimerUnitTest.java index 5a6db4d9c..f92cd4bcf 100644 --- a/public/java/test/org/broadinstitute/sting/utils/SimpleTimerUnitTest.java +++ b/public/java/test/org/broadinstitute/sting/utils/SimpleTimerUnitTest.java @@ -118,8 +118,8 @@ public class SimpleTimerUnitTest extends BaseTest { Assert.assertTrue(secs < 0.01, "Fast operation said to take longer than 10 milliseconds: elapsed time in seconds " + secs); Assert.assertTrue(nano > 0, "Nanosecond timer doesn't appear to count properly: elapsed time is " + nano); - final long maxTimeInMicro = 100; - final long maxTimeInNano = TimeUnit.MICROSECONDS.toNanos(100); + final long maxTimeInMicro = 10000; + final long maxTimeInNano = TimeUnit.MICROSECONDS.toNanos(maxTimeInMicro); Assert.assertTrue(nano < maxTimeInNano, "Fast operation said to take longer than " + maxTimeInMicro + " microseconds: elapsed time in nano " + nano + " micro " + TimeUnit.NANOSECONDS.toMicros(nano)); }