From 50b98a96e100133c3547b0004f1519e7385e4632 Mon Sep 17 00:00:00 2001 From: Daniel Cameron Date: Wed, 23 Jan 2019 21:54:53 +1100 Subject: [PATCH 1/5] Fixed memory leak in VCFCodec --- .../htsjdk/variant/variantcontext/Allele.java | 38 +++++++++++++++++-- .../htsjdk/variant/vcf/AbstractVCFCodec.java | 21 ++++++---- .../variantcontext/AlleleUnitTest.java | 30 +++++++++++++++ 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/main/java/htsjdk/variant/variantcontext/Allele.java b/src/main/java/htsjdk/variant/variantcontext/Allele.java index 0151a21af2..db9e4d2ad9 100644 --- a/src/main/java/htsjdk/variant/variantcontext/Allele.java +++ b/src/main/java/htsjdk/variant/variantcontext/Allele.java @@ -300,10 +300,36 @@ public static boolean wouldBeSymbolicAllele(final byte[] bases) { if ( bases.length <= 1 ) return false; else { - final String strBases = new String(bases); - return (bases[0] == '<' || bases[bases.length-1] == '>') || // symbolic or large insertion - (bases[0] == '.' || bases[bases.length-1] == '.') || // single breakend - (strBases.contains("[") || strBases.contains("]")); // mated breakend + return bases[0] == '<' || bases[bases.length - 1] == '>' || + wouldBeBreakpoint(bases) || + wouldBeSingleBreakend(bases); + } + } + /** + * @param bases bases representing an allele + * @return true if the bases represent a symbolic allele in breakpoint notation + */ + public static boolean wouldBeBreakpoint(final byte[] bases) { + if ( bases.length <= 1 ) + return false; + else { + for (int i = 0; i < bases.length; i++) { + if (bases[i] == '[' || bases[i] == ']') { + return true; + } + } + return false; + } + } + /** + * @param bases bases representing an allele + * @return true if the bases represent a symbolic allele in single breakend notation + */ + public static boolean wouldBeSingleBreakend(final byte[] bases) { + if ( bases.length <= 1 ) + return false; + else { + return bases[0] == '.' || bases[bases.length - 1] == '.'; } } @@ -421,6 +447,10 @@ public static Allele create(final Allele allele, final boolean ignoreRefState) { // Returns true if this Allele is symbolic (i.e. no well-defined base sequence) public boolean isSymbolic() { return isSymbolic; } + public boolean isBreakpoint() { return wouldBeBreakpoint(bases); } + + public boolean isSingleBreakend() { return wouldBeSingleBreakend(bases); } + // Returns a nice string representation of this object public String toString() { return ( isNoCall() ? NO_CALL_STRING : getDisplayString() ) + (isReference() ? "*" : ""); diff --git a/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java b/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java index 96d9b9c17f..97d6682eac 100644 --- a/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java +++ b/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java @@ -64,7 +64,9 @@ public abstract class AbstractVCFCodec extends AsciiFeatureCodec implements NameAwareCodec { public final static int MAX_ALLELE_SIZE_BEFORE_WARNING = (int)Math.pow(2, 20); - protected final static int NUM_STANDARD_FIELDS = 8; // INFO is the 8th column + protected final static int NUM_STANDARD_FIELDS = 8; // INFO is the 8th + + private final static int MAX_ALLELE_LENGTH_FOR_CACHING = 8; // we have to store the list of strings that make up the header until they're needed protected VCFHeader header = null; @@ -333,7 +335,13 @@ else if ( parts[2].equals(VCFConstants.EMPTY_ID_FIELD) ) builder.id(parts[2]); final String ref = getCachedString(parts[3].toUpperCase()); - final String alts = getCachedString(parts[4]); + String alts = parts[4]; + if (!(alts.contains(".") || alts.length() > MAX_ALLELE_LENGTH_FOR_CACHING || (alts.length() > 1 && (alts.contains("[") || alts.contains("]") || alts.contains("."))))) { + // don't cache multiple alt allele which we're going to strsplit anyway + // don't cache long alleles as they're probably unique + // don't cache breakpoint/single breakend alleles as they're probably unique + alts = getCachedString(alts); + } builder.log10PError(parseQual(parts[5])); final List filters = parseFilters(getCachedString(parts[6])); @@ -610,15 +618,14 @@ private static String generateExceptionTextForBadAlleleBases(final String allele } /** - * return true if this is a symbolic allele (e.g. ) or - * structural variation breakend (with [ or ]), otherwise false + * return true if this is a symbolic allele (e.g. ), + * structural variation breakend (with [ or ]), or single breakend (e.g. ATTA.), + * otherwise false * @param allele the allele to check * @return true if the allele is a symbolic allele, otherwise false */ private static boolean isSymbolicAllele(String allele) { - return (allele != null && allele.length() > 2 && - ((allele.startsWith("<") && allele.endsWith(">")) || - (allele.contains("[") || allele.contains("]")))); + return Allele.wouldBeSymbolicAllele(allele.getBytes()); } /** diff --git a/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java b/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java index e96c2680d8..8884bac1fb 100644 --- a/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java +++ b/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java @@ -277,4 +277,34 @@ public void testExtend() { Assert.assertEquals("ATCGA", Allele.extend(Allele.create("AT"), "CGA".getBytes()).toString()); Assert.assertEquals("ATCGA", Allele.extend(Allele.create("ATC"), "GA".getBytes()).toString()); } + @Test + public void testWouldBeSymbolic() { + Assert.assertTrue(Allele.wouldBeSymbolicAllele("".getBytes())); + Assert.assertTrue(Allele.wouldBeSymbolicAllele("AAAAAA[chr1:1234[".getBytes())); + Assert.assertTrue(Allele.wouldBeSymbolicAllele("AAAAAA]chr1:1234]".getBytes())); + Assert.assertTrue(Allele.wouldBeSymbolicAllele("A.".getBytes())); + Assert.assertTrue(Allele.wouldBeSymbolicAllele(".A".getBytes())); + Assert.assertFalse(Allele.wouldBeSymbolicAllele("AA".getBytes())); + Assert.assertFalse(Allele.wouldBeSymbolicAllele("A".getBytes())); + } + @Test + public void testWouldBeBreakpoint() { + Assert.assertFalse(Allele.wouldBeBreakpoint("".getBytes())); + Assert.assertTrue(Allele.wouldBeBreakpoint("AAAAAA[chr1:1234[".getBytes())); + Assert.assertTrue(Allele.wouldBeBreakpoint("AAAAAA]chr1:1234]".getBytes())); + Assert.assertFalse(Allele.wouldBeBreakpoint("A.".getBytes())); + Assert.assertFalse(Allele.wouldBeBreakpoint(".A".getBytes())); + Assert.assertFalse(Allele.wouldBeBreakpoint("AA".getBytes())); + Assert.assertFalse(Allele.wouldBeBreakpoint("A".getBytes())); + } + @Test + public void testWouldBeBreakend() { + Assert.assertFalse(Allele.wouldBeSingleBreakend("".getBytes())); + Assert.assertFalse(Allele.wouldBeSingleBreakend("AAAAAA[chr1:1234[".getBytes())); + Assert.assertFalse(Allele.wouldBeSingleBreakend("AAAAAA]chr1:1234]".getBytes())); + Assert.assertTrue(Allele.wouldBeSingleBreakend("A.".getBytes())); + Assert.assertTrue(Allele.wouldBeSingleBreakend(".A".getBytes())); + Assert.assertFalse(Allele.wouldBeSingleBreakend("AA".getBytes())); + Assert.assertFalse(Allele.wouldBeSingleBreakend("A".getBytes())); + } } From 5d993aa02fa000cb149b132444187383802477ec Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Wed, 13 Feb 2019 16:48:02 -0500 Subject: [PATCH 2/5] resolving my own comments --- .../htsjdk/variant/variantcontext/Allele.java | 21 +++++++++------ .../htsjdk/variant/vcf/AbstractVCFCodec.java | 27 +++---------------- .../variantcontext/AlleleUnitTest.java | 4 +++ 3 files changed, 21 insertions(+), 31 deletions(-) diff --git a/src/main/java/htsjdk/variant/variantcontext/Allele.java b/src/main/java/htsjdk/variant/variantcontext/Allele.java index db9e4d2ad9..0d3fb4750f 100644 --- a/src/main/java/htsjdk/variant/variantcontext/Allele.java +++ b/src/main/java/htsjdk/variant/variantcontext/Allele.java @@ -294,7 +294,7 @@ public static boolean wouldBeNoCallAllele(final byte[] bases) { /** * @param bases bases representing an allele - * @return true if the bases represent a symbolic allele + * @return true if the bases represent a symbolic allele, including breakpoints and breakends */ public static boolean wouldBeSymbolicAllele(final byte[] bases) { if ( bases.length <= 1 ) @@ -305,16 +305,18 @@ public static boolean wouldBeSymbolicAllele(final byte[] bases) { wouldBeSingleBreakend(bases); } } + /** * @param bases bases representing an allele - * @return true if the bases represent a symbolic allele in breakpoint notation + * @return true if the bases represent a symbolic allele in breakpoint notation, (ex: G]17:198982] or ]13:123456]T ) */ public static boolean wouldBeBreakpoint(final byte[] bases) { if ( bases.length <= 1 ) return false; else { for (int i = 0; i < bases.length; i++) { - if (bases[i] == '[' || bases[i] == ']') { + final byte base = bases[i]; + if (base == '[' || base == ']') { return true; } } @@ -323,7 +325,7 @@ public static boolean wouldBeBreakpoint(final byte[] bases) { } /** * @param bases bases representing an allele - * @return true if the bases represent a symbolic allele in single breakend notation + * @return true if the bases represent a symbolic allele in single breakend notation (ex: .A or A. ) */ public static boolean wouldBeSingleBreakend(final byte[] bases) { if ( bases.length <= 1 ) @@ -434,21 +436,24 @@ public static Allele create(final Allele allele, final boolean ignoreRefState) { // // --------------------------------------------------------------------------------------------------------- - // Returns true if this is the NO_CALL allele + /** @return true if this is the NO_CALL allele */ public boolean isNoCall() { return isNoCall; } // Returns true if this is not the NO_CALL allele public boolean isCalled() { return ! isNoCall(); } - // Returns true if this Allele is the reference allele + /** @return true if this Allele is the reference allele */ public boolean isReference() { return isRef; } - // Returns true if this Allele is not the reference allele + + /** @return true if this Allele is not the reference allele */ public boolean isNonReference() { return ! isReference(); } - // Returns true if this Allele is symbolic (i.e. no well-defined base sequence) + /** @return true if this Allele is symbolic (i.e. no well-defined base sequence), this includes breakpoints and breakends */ public boolean isSymbolic() { return isSymbolic; } + /** @return true if this Allele is a breakpoint ( ex: G]17:198982] or ]13:123456]T ) */ public boolean isBreakpoint() { return wouldBeBreakpoint(bases); } + /** @return true if this Allele is a single breakend (ex: .A or A.) */ public boolean isSingleBreakend() { return wouldBeSingleBreakend(bases); } // Returns a nice string representation of this object diff --git a/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java b/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java index 97d6682eac..3ae40f1653 100644 --- a/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java +++ b/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java @@ -66,8 +66,6 @@ public abstract class AbstractVCFCodec extends AsciiFeatureCodec protected final static int NUM_STANDARD_FIELDS = 8; // INFO is the 8th - private final static int MAX_ALLELE_LENGTH_FOR_CACHING = 8; - // we have to store the list of strings that make up the header until they're needed protected VCFHeader header = null; protected VCFHeaderVersion version = null; @@ -334,18 +332,12 @@ else if ( parts[2].equals(VCFConstants.EMPTY_ID_FIELD) ) else builder.id(parts[2]); - final String ref = getCachedString(parts[3].toUpperCase()); - String alts = parts[4]; - if (!(alts.contains(".") || alts.length() > MAX_ALLELE_LENGTH_FOR_CACHING || (alts.length() > 1 && (alts.contains("[") || alts.contains("]") || alts.contains("."))))) { - // don't cache multiple alt allele which we're going to strsplit anyway - // don't cache long alleles as they're probably unique - // don't cache breakpoint/single breakend alleles as they're probably unique - alts = getCachedString(alts); - } + final String ref = parts[3].toUpperCase(); + final String alts = parts[4]; builder.log10PError(parseQual(parts[5])); final List filters = parseFilters(getCachedString(parts[6])); - if ( filters != null ) builder.filters(new HashSet(filters)); + if ( filters != null ) builder.filters(new HashSet<>(filters)); final Map attrs = parseInfo(parts[7]); builder.attributes(attrs); @@ -585,7 +577,7 @@ private static void checkAllele(String allele, boolean isRef, int lineNo) { System.err.println(String.format("Allele detected with length %d exceeding max size %d at approximately line %d, likely resulting in degraded VCF processing performance", allele.length(), MAX_ALLELE_SIZE_BEFORE_WARNING, lineNo)); } - if ( isSymbolicAllele(allele) ) { + if (Allele.wouldBeSymbolicAllele(allele.getBytes())) { if ( isRef ) { generateException("Symbolic alleles not allowed as reference allele: " + allele, lineNo); } @@ -617,17 +609,6 @@ private static String generateExceptionTextForBadAlleleBases(final String allele return "unparsable vcf record with allele " + allele; } - /** - * return true if this is a symbolic allele (e.g. ), - * structural variation breakend (with [ or ]), or single breakend (e.g. ATTA.), - * otherwise false - * @param allele the allele to check - * @return true if the allele is a symbolic allele, otherwise false - */ - private static boolean isSymbolicAllele(String allele) { - return Allele.wouldBeSymbolicAllele(allele.getBytes()); - } - /** * parse a single allele, given the allele list * @param alleles the alleles available diff --git a/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java b/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java index 8884bac1fb..b302110c70 100644 --- a/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java +++ b/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java @@ -289,6 +289,8 @@ public void testWouldBeSymbolic() { } @Test public void testWouldBeBreakpoint() { + Assert.assertTrue(Allele.wouldBeBreakpoint("G]17:198982]".getBytes())); + Assert.assertTrue(Allele.wouldBeBreakpoint("]13:123456]T".getBytes())); Assert.assertFalse(Allele.wouldBeBreakpoint("".getBytes())); Assert.assertTrue(Allele.wouldBeBreakpoint("AAAAAA[chr1:1234[".getBytes())); Assert.assertTrue(Allele.wouldBeBreakpoint("AAAAAA]chr1:1234]".getBytes())); @@ -299,6 +301,8 @@ public void testWouldBeBreakpoint() { } @Test public void testWouldBeBreakend() { + Assert.assertFalse(Allele.wouldBeSingleBreakend("G]17:198982]".getBytes())); + Assert.assertFalse(Allele.wouldBeSingleBreakend("]13:123456]T".getBytes())); Assert.assertFalse(Allele.wouldBeSingleBreakend("".getBytes())); Assert.assertFalse(Allele.wouldBeSingleBreakend("AAAAAA[chr1:1234[".getBytes())); Assert.assertFalse(Allele.wouldBeSingleBreakend("AAAAAA]chr1:1234]".getBytes())); From 77466a1d65e373541dabc733152573cdb462b500 Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Thu, 21 Feb 2019 15:00:19 -0500 Subject: [PATCH 3/5] Apply suggestions from code review spacing fix Co-Authored-By: lbergelson --- src/main/java/htsjdk/variant/variantcontext/Allele.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/htsjdk/variant/variantcontext/Allele.java b/src/main/java/htsjdk/variant/variantcontext/Allele.java index 0d3fb4750f..56b3bf590c 100644 --- a/src/main/java/htsjdk/variant/variantcontext/Allele.java +++ b/src/main/java/htsjdk/variant/variantcontext/Allele.java @@ -311,7 +311,7 @@ public static boolean wouldBeSymbolicAllele(final byte[] bases) { * @return true if the bases represent a symbolic allele in breakpoint notation, (ex: G]17:198982] or ]13:123456]T ) */ public static boolean wouldBeBreakpoint(final byte[] bases) { - if ( bases.length <= 1 ) + if (bases.length <= 1) { return false; else { for (int i = 0; i < bases.length; i++) { From ed5991ee31e6331f4deb5669f32f3919b0552eb5 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Thu, 21 Feb 2019 16:54:20 -0500 Subject: [PATCH 4/5] responding to comments --- .../htsjdk/variant/variantcontext/Allele.java | 22 +++-- .../htsjdk/variant/vcf/AbstractVCFCodec.java | 4 +- .../variantcontext/AlleleUnitTest.java | 82 ++++++++++--------- 3 files changed, 60 insertions(+), 48 deletions(-) diff --git a/src/main/java/htsjdk/variant/variantcontext/Allele.java b/src/main/java/htsjdk/variant/variantcontext/Allele.java index 56b3bf590c..387a8f7437 100644 --- a/src/main/java/htsjdk/variant/variantcontext/Allele.java +++ b/src/main/java/htsjdk/variant/variantcontext/Allele.java @@ -118,6 +118,11 @@ public class Allele implements Comparable, Serializable { public static final long serialVersionUID = 1L; private static final byte[] EMPTY_ALLELE_BASES = new byte[0]; + private static final char SINGLE_BREAKEND_INDICATOR = '.'; + private static final char BREAKEND_EXTENDING_RIGHT = '['; + private static final char BREAKEND_EXTENDING_LEFT = ']'; + private static final char SYMBOLIC_ALLELE_START = '<'; + private static final char SYMBOLIC_ALLELE_END = '>'; private boolean isRef = false; private boolean isNoCall = false; @@ -300,7 +305,7 @@ public static boolean wouldBeSymbolicAllele(final byte[] bases) { if ( bases.length <= 1 ) return false; else { - return bases[0] == '<' || bases[bases.length - 1] == '>' || + return bases[0] == SYMBOLIC_ALLELE_START || bases[bases.length - 1] == SYMBOLIC_ALLELE_END || wouldBeBreakpoint(bases) || wouldBeSingleBreakend(bases); } @@ -313,15 +318,14 @@ public static boolean wouldBeSymbolicAllele(final byte[] bases) { public static boolean wouldBeBreakpoint(final byte[] bases) { if (bases.length <= 1) { return false; - else { - for (int i = 0; i < bases.length; i++) { - final byte base = bases[i]; - if (base == '[' || base == ']') { - return true; - } + } + for (int i = 0; i < bases.length; i++) { + final byte base = bases[i]; + if (base == BREAKEND_EXTENDING_LEFT || base == BREAKEND_EXTENDING_RIGHT) { + return true; } - return false; } + return false; } /** * @param bases bases representing an allele @@ -331,7 +335,7 @@ public static boolean wouldBeSingleBreakend(final byte[] bases) { if ( bases.length <= 1 ) return false; else { - return bases[0] == '.' || bases[bases.length - 1] == '.'; + return bases[0] == SINGLE_BREAKEND_INDICATOR || bases[bases.length - 1] == SINGLE_BREAKEND_INDICATOR; } } diff --git a/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java b/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java index 3ae40f1653..bfa48b4c87 100644 --- a/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java +++ b/src/main/java/htsjdk/variant/vcf/AbstractVCFCodec.java @@ -337,7 +337,9 @@ else if ( parts[2].equals(VCFConstants.EMPTY_ID_FIELD) ) builder.log10PError(parseQual(parts[5])); final List filters = parseFilters(getCachedString(parts[6])); - if ( filters != null ) builder.filters(new HashSet<>(filters)); + if ( filters != null ) { + builder.filters(new HashSet<>(filters)); + } final Map attrs = parseInfo(parts[7]); builder.attributes(attrs); diff --git a/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java b/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java index b302110c70..8770d8d425 100644 --- a/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java +++ b/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java @@ -32,6 +32,7 @@ import org.testng.Assert; import org.testng.annotations.BeforeSuite; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; // public Allele(byte[] bases, boolean isRef) { @@ -269,46 +270,51 @@ public void testBadSpanningDeletionAllelel() { Allele.create(Allele.SPAN_DEL_STRING, true); // spanning deletion cannot be ref allele } - @Test - public void testExtend() { - Assert.assertEquals("AT", Allele.extend(Allele.create("A"), "T".getBytes()).toString()); - Assert.assertEquals("ATA", Allele.extend(Allele.create("A"), "TA".getBytes()).toString()); - Assert.assertEquals("A", Allele.extend(Allele.NO_CALL, "A".getBytes()).toString()); - Assert.assertEquals("ATCGA", Allele.extend(Allele.create("AT"), "CGA".getBytes()).toString()); - Assert.assertEquals("ATCGA", Allele.extend(Allele.create("ATC"), "GA".getBytes()).toString()); + @DataProvider + public Object[][] getExtendTests() { + return new Object[][]{ + {Allele.create("A"), "T", "AT"}, + {Allele.create("A"), "TA", "ATA"}, + {Allele.NO_CALL, "A", "A"}, + {Allele.create("AT"), "CGA", "ATCGA"}, + {Allele.create("ATC"), "GA", "ATCGA"} + }; } - @Test - public void testWouldBeSymbolic() { - Assert.assertTrue(Allele.wouldBeSymbolicAllele("".getBytes())); - Assert.assertTrue(Allele.wouldBeSymbolicAllele("AAAAAA[chr1:1234[".getBytes())); - Assert.assertTrue(Allele.wouldBeSymbolicAllele("AAAAAA]chr1:1234]".getBytes())); - Assert.assertTrue(Allele.wouldBeSymbolicAllele("A.".getBytes())); - Assert.assertTrue(Allele.wouldBeSymbolicAllele(".A".getBytes())); - Assert.assertFalse(Allele.wouldBeSymbolicAllele("AA".getBytes())); - Assert.assertFalse(Allele.wouldBeSymbolicAllele("A".getBytes())); + + @Test(dataProvider = "getExtendTests") + public void testExtend(Allele toExtend, String extension, String expected) { + final Allele extended = Allele.extend(toExtend, extension.getBytes()); + Assert.assertEquals(extended, Allele.create(expected)); } - @Test - public void testWouldBeBreakpoint() { - Assert.assertTrue(Allele.wouldBeBreakpoint("G]17:198982]".getBytes())); - Assert.assertTrue(Allele.wouldBeBreakpoint("]13:123456]T".getBytes())); - Assert.assertFalse(Allele.wouldBeBreakpoint("".getBytes())); - Assert.assertTrue(Allele.wouldBeBreakpoint("AAAAAA[chr1:1234[".getBytes())); - Assert.assertTrue(Allele.wouldBeBreakpoint("AAAAAA]chr1:1234]".getBytes())); - Assert.assertFalse(Allele.wouldBeBreakpoint("A.".getBytes())); - Assert.assertFalse(Allele.wouldBeBreakpoint(".A".getBytes())); - Assert.assertFalse(Allele.wouldBeBreakpoint("AA".getBytes())); - Assert.assertFalse(Allele.wouldBeBreakpoint("A".getBytes())); + + @DataProvider + public Object[][] getTestCasesForCheckingSymbolicAlleles(){ + return new Object[][]{ + //allele, isSymbolic, isBreakpoint, isSingleBreakend + {"", true, false, false}, + {"G]17:198982]", true, true, false}, + {"]13:123456]T", true, true, false}, + {"AAAAAA[chr1:1234[", true, true, false}, + {"AAAAAA]chr1:1234]", true, true, false}, + {"A.", true, false, true}, + {".A", false, true, true}, + {"AA", false, false, false}, + {"A", false, false, false} + }; } - @Test - public void testWouldBeBreakend() { - Assert.assertFalse(Allele.wouldBeSingleBreakend("G]17:198982]".getBytes())); - Assert.assertFalse(Allele.wouldBeSingleBreakend("]13:123456]T".getBytes())); - Assert.assertFalse(Allele.wouldBeSingleBreakend("".getBytes())); - Assert.assertFalse(Allele.wouldBeSingleBreakend("AAAAAA[chr1:1234[".getBytes())); - Assert.assertFalse(Allele.wouldBeSingleBreakend("AAAAAA]chr1:1234]".getBytes())); - Assert.assertTrue(Allele.wouldBeSingleBreakend("A.".getBytes())); - Assert.assertTrue(Allele.wouldBeSingleBreakend(".A".getBytes())); - Assert.assertFalse(Allele.wouldBeSingleBreakend("AA".getBytes())); - Assert.assertFalse(Allele.wouldBeSingleBreakend("A".getBytes())); + + @Test(dataProvider = "getTestCasesForCheckingSymbolicAlleles") + public void testWouldBeSymbolic(String baseString, boolean isSymbolic, boolean isBreakpoint, boolean isBreakend) { + Assert.assertEquals(Allele.wouldBeSymbolicAllele(baseString.getBytes()), isSymbolic); + } + + @Test(dataProvider = "getTestCasesForCheckingSymbolicAlleles") + public void testWouldBeBreakpoint(String baseString, boolean isSymbolic, boolean isBreakpoint, boolean isBreakend) { + Assert.assertEquals(Allele.wouldBeBreakpoint(baseString.getBytes()), isBreakpoint); + } + + @Test(dataProvider = "getTestCasesForCheckingSymbolicAlleles") + public void testWouldBeBreakend(String baseString, boolean isSymbolic, boolean isBreakpoint, boolean isBreakend) { + Assert.assertEquals(Allele.wouldBeSingleBreakend(baseString.getBytes()), isBreakend); } } From 2862c14d373772c9a1401de222b9e8fc29413fad Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Mon, 25 Feb 2019 14:46:31 -0500 Subject: [PATCH 5/5] fix stupid thing --- src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java b/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java index 8770d8d425..2de5373f19 100644 --- a/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java +++ b/src/test/java/htsjdk/variant/variantcontext/AlleleUnitTest.java @@ -297,7 +297,7 @@ public Object[][] getTestCasesForCheckingSymbolicAlleles(){ {"AAAAAA[chr1:1234[", true, true, false}, {"AAAAAA]chr1:1234]", true, true, false}, {"A.", true, false, true}, - {".A", false, true, true}, + {".A", true, false, true}, {"AA", false, false, false}, {"A", false, false, false} };