From edfd6f8a06bfbaaddfbb26677dc98f33af9f6318 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Thu, 13 Oct 2011 21:32:52 -0400 Subject: [PATCH 1/6] Removing a public -> private dependency from the test suite. The public integration test VariantContextIntegrationTest was dependent on the private walker TestVariantContextWalker. Moved this walker to public/java/test (NOT public/java/src, since this walker is only used by the test suite) to avoid errors during public-only tests. --- .../walkers/qc/TestVariantContextWalker.java | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100755 public/java/test/org/broadinstitute/sting/gatk/walkers/qc/TestVariantContextWalker.java diff --git a/public/java/test/org/broadinstitute/sting/gatk/walkers/qc/TestVariantContextWalker.java b/public/java/test/org/broadinstitute/sting/gatk/walkers/qc/TestVariantContextWalker.java new file mode 100755 index 000000000..7607049db --- /dev/null +++ b/public/java/test/org/broadinstitute/sting/gatk/walkers/qc/TestVariantContextWalker.java @@ -0,0 +1,132 @@ +/* + * Copyright (c) 2010 The Broad Institute + * + * Permission is hereby granted, free of charge, to any person + * obtaining a copy of this software and associated documentation + * files (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following + * conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR + * THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +package org.broadinstitute.sting.gatk.walkers.qc; + +import org.broadinstitute.sting.commandline.Argument; +import org.broadinstitute.sting.commandline.ArgumentCollection; +import org.broadinstitute.sting.commandline.Output; +import org.broadinstitute.sting.gatk.arguments.DbsnpArgumentCollection; +import org.broadinstitute.sting.gatk.arguments.StandardVariantContextInputArgumentCollection; +import org.broadinstitute.sting.gatk.contexts.AlignmentContext; +import org.broadinstitute.sting.gatk.contexts.ReferenceContext; +import org.broadinstitute.sting.gatk.refdata.RefMetaDataTracker; +import org.broadinstitute.sting.gatk.refdata.VariantContextAdaptors; +import org.broadinstitute.sting.gatk.walkers.Reference; +import org.broadinstitute.sting.gatk.walkers.RodWalker; +import org.broadinstitute.sting.gatk.walkers.Window; +import org.broadinstitute.sting.utils.codecs.vcf.VCFWriter; +import org.broadinstitute.sting.utils.variantcontext.VariantContext; + +import java.io.PrintStream; +import java.util.Arrays; +import java.util.EnumSet; +import java.util.List; + +/** + * Test routine for new VariantContext object + */ +@Reference(window=@Window(start=-20,stop=1)) +public class TestVariantContextWalker extends RodWalker { + @Output + PrintStream out; + + @ArgumentCollection + protected StandardVariantContextInputArgumentCollection variantCollection = new StandardVariantContextInputArgumentCollection(); + + @Argument(fullName="takeFirstOnly", doc="Only take the first second at a locus, as opposed to all", required=false) + boolean takeFirstOnly = false; + + @Argument(fullName="onlyContextsOfType", doc="Only take variant contexts of this type", required=false) + VariantContext.Type onlyOfThisType = null; + + @Argument(fullName="onlyContextsStartinAtCurrentPosition", doc="Only take variant contexts at actually start at the current position, excluding those at span to the current location but start earlier", required=false) + boolean onlyContextsStartinAtCurrentPosition = false; + + @Argument(fullName="printPerLocus", doc="If true, we'll print the variant contexts, in addition to counts", required=false) + boolean printContexts = false; + + @Argument(fullName="outputVCF", doc="If provided, we'll convert the first input context into a VCF", required=false) + VCFWriter writer = null; + + private boolean wroteHeader = false; + + public Integer map(RefMetaDataTracker tracker, ReferenceContext ref, AlignmentContext context) { + if ( ref == null ) + return 0; + else { + EnumSet allowedTypes = onlyOfThisType == null ? null : EnumSet.of(onlyOfThisType); + + int n = 0; + List contexts; + if ( onlyContextsStartinAtCurrentPosition ) + contexts = tracker.getValues(variantCollection.variants, context.getLocation()); + else // ! onlyContextsStartinAtCurrentPosition + contexts = tracker.getValues(variantCollection.variants); + + for ( VariantContext vc : contexts ) { + if ( allowedTypes == null || allowedTypes.contains(vc.getType()) ) { + // we need to trigger decoding of the genotype string to pass integration tests + vc.getGenotypes(); + + if ( writer != null && n == 0 ) { + if ( ! wroteHeader ) { + writer.writeHeader(VariantContextAdaptors.createVCFHeader(null, vc)); + wroteHeader = true; + } + + writer.add(vc); + } + + n++; + if ( printContexts ) out.printf(" %s%n", vc); + if ( takeFirstOnly ) break; + } + } + + if ( n > 0 && printContexts ) { + out.printf("%s => had %d variant context objects%n", context.getLocation(), n); + out.printf("---------------------------------------------%n"); + } + + return n; + } + } + + public Integer reduceInit() { + return 0; + } + + public Integer reduce(Integer point, Integer sum) { + return point + sum; + } + + @Override + public void onTraversalDone(Integer result) { + // Double check traversal result to make count is the same. + // TODO: Is this check necessary? + out.println("[REDUCE RESULT] Traversal result is: " + result); + } +} From 442d33ba186d0c008a83b0da3b20dec0c91f070a Mon Sep 17 00:00:00 2001 From: David Roazen Date: Wed, 28 Sep 2011 16:23:54 -0400 Subject: [PATCH 2/6] Enable testing of the jars produced by the packaging system. -Added targets to run unit and integration tests on the fully-packaged GATK jar, and pipeline tests on the fully-packaged Queue jar. Once enabled in Bamboo, these will provide greatly-enhanced protection against breakage in the binary release. -Unconditionally include all of the subset of org.broadinstitute.sting included in the intermediate jars GenomeAnalysisTK.jar, StingUtils.jar, etc. in the final, fully-packaged jar. This: * is necessary to get tests to run on the fully-packaged jar * decreases the chances of a class that is a runtime-only dependency getting left out of the binary release * only slightly increases the size of the binary release (before: 9352465 bytes, after: 10985482 bytes) --- build.xml | 156 ++++++++++++++++++++++++--------- public/packages/GATKEngine.xml | 10 +-- 2 files changed, 116 insertions(+), 50 deletions(-) diff --git a/build.xml b/build.xml index a4caa96c9..16bc8d9b4 100644 --- a/build.xml +++ b/build.xml @@ -67,6 +67,8 @@ + + @@ -118,6 +120,16 @@ + + + + + + + + + + @@ -827,10 +839,54 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -851,6 +907,7 @@ - - - - - - - - - - @@ -902,6 +949,10 @@ + + + + @@ -914,62 +965,81 @@ + + + + + + + + + + + + + + + + + + + - + - + - + - + - + - + - + - + - + - + - - + + - - + + - - + + - - + + @@ -1027,14 +1097,14 @@ - + - - + + @@ -1043,8 +1113,8 @@ - - + + @@ -1055,11 +1125,11 @@ - + - + @@ -1068,7 +1138,7 @@ - + @@ -1076,14 +1146,14 @@ - - - + + + - - - + + + @@ -1119,7 +1189,7 @@ - + diff --git a/public/packages/GATKEngine.xml b/public/packages/GATKEngine.xml index 4364988e7..e6056408e 100644 --- a/public/packages/GATKEngine.xml +++ b/public/packages/GATKEngine.xml @@ -26,14 +26,10 @@ - - - - - - + + - + From bd8bb93811aa3ec20d6777dce84c6a62b8d35f3d Mon Sep 17 00:00:00 2001 From: David Roazen Date: Fri, 14 Oct 2011 20:04:42 -0400 Subject: [PATCH 3/6] Split RScriptExecutorUnitTest into public and private test classes. We can't have a public test that depends on both public and private code/data -- the new release system needs to do public-only tests, and will catch this sort of thing. --- .../utils/R/RScriptExecutorUnitTest.java | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/public/java/test/org/broadinstitute/sting/utils/R/RScriptExecutorUnitTest.java b/public/java/test/org/broadinstitute/sting/utils/R/RScriptExecutorUnitTest.java index 1bbf74db9..836a4473f 100644 --- a/public/java/test/org/broadinstitute/sting/utils/R/RScriptExecutorUnitTest.java +++ b/public/java/test/org/broadinstitute/sting/utils/R/RScriptExecutorUnitTest.java @@ -26,21 +26,12 @@ package org.broadinstitute.sting.utils.R; import org.apache.commons.io.FileUtils; import org.broadinstitute.sting.BaseTest; -import org.broadinstitute.sting.gatk.walkers.diffengine.DiffElement; -import org.broadinstitute.sting.gatk.walkers.diffengine.DiffEngine; -import org.broadinstitute.sting.gatk.walkers.diffengine.DiffNode; -import org.broadinstitute.sting.gatk.walkers.diffengine.Difference; import org.broadinstitute.sting.utils.exceptions.UserException; -import org.testng.Assert; -import org.testng.annotations.BeforeClass; -import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.io.File; import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.List; /** @@ -49,7 +40,6 @@ import java.util.List; public class RScriptExecutorUnitTest extends BaseTest { final static String testrscript = "print(\"hello, world\")\n"; final static String publicRScript = "plot_Tranches.R"; - final static String privateRScript = "variantCallQC.R"; // -------------------------------------------------------------------------------- // @@ -74,17 +64,6 @@ public class RScriptExecutorUnitTest extends BaseTest { @Test public void testPublic() { testOne(publicRScript, null, null, true); } - @Test - public void testPrivate() { testOne(privateRScript, null, null, true); } - - // make sure we don't break finding something in private by adding another directory - @Test - public void testPrivateWithAdditionalPath1() { testOne(privateRScript, null, "dist", true); } - - // make sure we don't break finding something in private by adding another directory - @Test - public void testPrivateWithAdditionalPath2() { testOne(privateRScript, null, "doesNotExist", true); } - @Test(expectedExceptions = UserException.class) public void testNonExistantScriptException() { testOne("does_not_exist.R", null, null, true); } From c756da179888dbca8d157330f14f7737d596f89a Mon Sep 17 00:00:00 2001 From: David Roazen Date: Fri, 14 Oct 2011 22:59:12 -0400 Subject: [PATCH 4/6] Remove unnecessary debugger port listening from GATKDoclet invocation. This caused a race condition between concurrently-running bamboo plans. --- build.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.xml b/build.xml index 16bc8d9b4..babf31506 100644 --- a/build.xml +++ b/build.xml @@ -519,7 +519,7 @@ docletpathref="doclet.classpath" classpathref="external.dependencies" classpath="${java.classes}" - additionalparam="${gatkdocs.include.hidden.arg} -private -build-timestamp "${build.timestamp}" -absolute-version ${build.version} -quiet -J-Xdebug -J-Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=5005"> + additionalparam="${gatkdocs.include.hidden.arg} -private -build-timestamp "${build.timestamp}" -absolute-version ${build.version} -quiet"> From 46badee76a16073b286d3809f900857e0fc3fb7a Mon Sep 17 00:00:00 2001 From: David Roazen Date: Sat, 15 Oct 2011 20:20:41 -0400 Subject: [PATCH 5/6] Intentionally breaking the binary release as a test. This is to test the ability of the new release system to detect the kinds of packaging errors we couldn't detect before (in this case, missing codecs). If all goes as expected, "GSA-Stable: Release" will fail, preventing the binary release and github from getting updated, while "GSA-Stable: All Tests" and all of the other old test plans will pass. Will revert this in a bit -- if the system works as it should, our users will never see it until after it's been reverted. --- build.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.xml b/build.xml index babf31506..75eaa3ffb 100644 --- a/build.xml +++ b/build.xml @@ -541,7 +541,7 @@ - + From dcd4eee15f7eb350a25f6e2c1fabd89b6e95b47c Mon Sep 17 00:00:00 2001 From: David Roazen Date: Sun, 16 Oct 2011 07:24:33 -0400 Subject: [PATCH 6/6] Revert "Intentionally breaking the binary release as a test." The test was successful -- the packaging error was detected and prevented from propagating out into the binary/source releases. This reverts commit 8f2a1462d94ce50fb7c1b1d0c40142b109e9c38e. --- build.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.xml b/build.xml index 75eaa3ffb..babf31506 100644 --- a/build.xml +++ b/build.xml @@ -541,7 +541,7 @@ - +