From 8076e2620af4924e938fb5f0fb1a7619b6136dcd Mon Sep 17 00:00:00 2001 From: Yossi Farjoun Date: Thu, 5 Jan 2017 15:58:55 -0500 Subject: [PATCH 1/4] - added tests to the most eggregious classes (regarding test coverage) I could find. - found a tiny bug in the logic of overlaps in the Node class. --- src/main/java/htsjdk/samtools/util/DateParser.java | 41 ------ .../java/htsjdk/samtools/util/IntervalTree.java | 2 +- .../java/htsjdk/samtools/util/SequenceUtil.java | 35 ++--- .../java/htsjdk/samtools/BAMFileIndexTest.java | 15 +++ .../java/htsjdk/samtools/util/DateParserTest.java | 146 +++++++++++++++++++++ .../htsjdk/samtools/util/IntervalTreeMapTest.java | 4 +- .../htsjdk/samtools/util/IntervalTreeTest.java | 94 ++++++++++++- .../htsjdk/samtools/util/SequenceUtilTest.java | 138 ++++++++++++++++++- 8 files changed, 399 insertions(+), 76 deletions(-) create mode 100644 src/test/java/htsjdk/samtools/util/DateParserTest.java diff --git a/src/main/java/htsjdk/samtools/util/DateParser.java b/src/main/java/htsjdk/samtools/util/DateParser.java index 02a960986..f2d9481c7 100644 --- a/src/main/java/htsjdk/samtools/util/DateParser.java +++ b/src/main/java/htsjdk/samtools/util/DateParser.java @@ -277,47 +277,6 @@ public static String getIsoDate(Date date) { .append("Z").toString(); } - public static void test(String isodate) { - System.out.println("----------------------------------"); - try { - Date date = parse(isodate); - System.out.println(">> "+isodate); - System.out.println(">> "+date.toString()+" ["+date.getTime()+"]"); - System.out.println(">> "+getIsoDate(date)); - } catch (InvalidDateException ex) { - System.err.println(isodate+" is invalid"); - System.err.println(ex.getMessage()); - } - System.out.println("----------------------------------"); - } - - public static void test(Date date) { - String isodate = null; - System.out.println("----------------------------------"); - try { - System.out.println(">> "+date.toString()+" ["+date.getTime()+"]"); - isodate = getIsoDate(date); - System.out.println(">> "+isodate); - date = parse(isodate); - System.out.println(">> "+date.toString()+" ["+date.getTime()+"]"); - } catch (InvalidDateException ex) { - System.err.println(isodate+" is invalid"); - System.err.println(ex.getMessage()); - } - System.out.println("----------------------------------"); - } - - public static void main(String args[]) { - test("1997-07-16T19:20:30.45-02:00"); - test("1997-07-16T19:20:30+01:00"); - test("1997-07-16T19:20:30+01:00"); - test("1997-07-16T19:20"); - test("1997-07-16"); - test("1997-07"); - test("1997"); - test(new Date()); - } - public static class InvalidDateException extends SAMException { public InvalidDateException() { } diff --git a/src/main/java/htsjdk/samtools/util/IntervalTree.java b/src/main/java/htsjdk/samtools/util/IntervalTree.java index 49c3017e1..be4ae130e 100644 --- a/src/main/java/htsjdk/samtools/util/IntervalTree.java +++ b/src/main/java/htsjdk/samtools/util/IntervalTree.java @@ -492,7 +492,7 @@ public int getRelationship( final Node interval ) result = HAS_LESSER_PART; if ( mEnd > interval.getEnd() ) result |= HAS_GREATER_PART; - if ( mStart < interval.getEnd() && interval.getStart() < mEnd ) + if ( mStart <= interval.getEnd() && interval.getStart() <= mEnd ) result |= HAS_OVERLAPPING_PART; return result; } diff --git a/src/main/java/htsjdk/samtools/util/SequenceUtil.java b/src/main/java/htsjdk/samtools/util/SequenceUtil.java index 3108cee0d..92c1a507d 100644 --- a/src/main/java/htsjdk/samtools/util/SequenceUtil.java +++ b/src/main/java/htsjdk/samtools/util/SequenceUtil.java @@ -620,32 +620,7 @@ public static byte complement(final byte b) { } } - /** Reverses and complements the bases in place. */ - public static void reverseComplement(final byte[] bases) { - final int lastIndex = bases.length - 1; - int i, j; - for (i = 0, j = lastIndex; i < j; ++i, --j) { - final byte tmp = complement(bases[i]); - bases[i] = complement(bases[j]); - bases[j] = tmp; - } - if (bases.length % 2 == 1) { - bases[i] = complement(bases[i]); - } - } - - /** Reverses the quals in place. */ - public static void reverseQualities(final byte[] quals) { - final int lastIndex = quals.length - 1; - - int i, j; - for (i = 0, j = lastIndex; i < j; ++i, --j) { - final byte tmp = quals[i]; - quals[i] = quals[j]; - quals[j] = tmp; - } - } /** * Returns true if the bases are equal OR if the mismatch can be accounted for by @@ -836,6 +811,16 @@ else if (cigElOp.consumesReferenceBases()) { return ret; } + /** Reverses and complements the bases in place. */ + public static void reverseComplement(final byte[] bases) { + reverseComplement(bases, 0, bases.length); + } + + /** Reverses the quals in place. */ + public static void reverseQualities(final byte[] quals) { + reverse(quals, 0, quals.length); + } + public static void reverse(final byte[] array, final int offset, final int len) { final int lastIndex = len - 1; diff --git a/src/test/java/htsjdk/samtools/BAMFileIndexTest.java b/src/test/java/htsjdk/samtools/BAMFileIndexTest.java index 170bc4726..33c3709c0 100755 --- a/src/test/java/htsjdk/samtools/BAMFileIndexTest.java +++ b/src/test/java/htsjdk/samtools/BAMFileIndexTest.java @@ -181,6 +181,21 @@ public void testQueryAlignmentStart() { CloserUtil.close(reader); } + @DataProvider(name = "queryIntervalsData") + public Object[][] queryIntervalsData(){ + return new Object[][] { + {true, 1}, + {false, 2} + }; + } + @Test(dataProvider = "queryIntervalsData") + public void testQueryIntervals(final boolean contained, final int expected) { + final SamReader reader = SamReaderFactory.makeDefault().enable().open(BAM_FILE); + + final CloseableIterator it = reader.query("chr1", 202661637, 202661812, contained); + Assert.assertEquals(countElements(it), expected); + } + @Test public void testQueryMate() { final SamReader reader = SamReaderFactory.makeDefault().open(BAM_FILE); diff --git a/src/test/java/htsjdk/samtools/util/DateParserTest.java b/src/test/java/htsjdk/samtools/util/DateParserTest.java new file mode 100644 index 000000000..4a28cc6d6 --- /dev/null +++ b/src/test/java/htsjdk/samtools/util/DateParserTest.java @@ -0,0 +1,146 @@ +// DateParser.java +// $Id: DateParser.java,v 1.3 2001/01/04 13:26:19 bmahe Exp $ +// (c) COPYRIGHT MIT, INRIA and Keio, 2000. + +/* +W3C IPR SOFTWARE NOTICE + +Copyright 1995-1998 World Wide Web Consortium, (Massachusetts Institute of +Technology, Institut National de Recherche en Informatique et en +Automatique, Keio University). All Rights Reserved. +http://www.w3.org/Consortium/Legal/ + +This W3C work (including software, documents, or other related items) is +being provided by the copyright holders under the following license. By +obtaining, using and/or copying this work, you (the licensee) agree that you +have read, understood, and will comply with the following terms and +conditions: + +Permission to use, copy, and modify this software and its documentation, +with or without modification, for any purpose and without fee or royalty is +hereby granted, provided that you include the following on ALL copies of the +software and documentation or portions thereof, including modifications, +that you make: + + 1. The full text of this NOTICE in a location viewable to users of the + redistributed or derivative work. + 2. Any pre-existing intellectual property disclaimers, notices, or terms + and conditions. If none exist, a short notice of the following form + (hypertext is preferred, text is permitted) should be used within the + body of any redistributed or derivative code: "Copyright World Wide + Web Consortium, (Massachusetts Institute of Technology, Institut + National de Recherche en Informatique et en Automatique, Keio + University). All Rights Reserved. http://www.w3.org/Consortium/Legal/" + 3. Notice of any changes or modifications to the W3C files, including the + date changes were made. (We recommend you provide URIs to the location + from which the code is derived). + +In addition, creators of derivitive works must include the full text of this +NOTICE in a location viewable to users of the derivitive work. + +THIS SOFTWARE AND DOCUMENTATION IS PROVIDED "AS IS," AND COPYRIGHT HOLDERS +MAKE NO REPRESENTATIONS OR WARRANTIES, EXPRESS OR IMPLIED, INCLUDING BUT NOT +LIMITED TO, WARRANTIES OF MERCHANTABILITY OR FITNESS FOR ANY PARTICULAR +PURPOSE OR THAT THE USE OF THE SOFTWARE OR DOCUMENTATION WILL NOT INFRINGE +ANY THIRD PARTY PATENTS, COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS. + +COPYRIGHT HOLDERS WILL NOT BE LIABLE FOR ANY DIRECT, INDIRECT, SPECIAL OR +CONSEQUENTIAL DAMAGES ARISING OUT OF ANY USE OF THE SOFTWARE OR +DOCUMENTATION. + +The name and trademarks of copyright holders may NOT be used in advertising +or publicity pertaining to the software without specific, written prior +permission. Title to copyright in this software and any associated +documentation will at all times remain with copyright holders. + +____________________________________ + +This formulation of W3C's notice and license became active on August 14 +1998. See the older formulation for the policy prior to this date. Please +see our Copyright FAQ for common questions about using materials from our +site, including specific terms and conditions for packages like libwww, +Amaya, and Jigsaw. Other questions about this notice can be directed to +site-policy@w3.org . + + + + +webmaster +(last updated 14-Aug-1998) + + */ + +package htsjdk.samtools.util; + +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import java.util.Date; + +/** + * NOTE: This code has been taken from w3.org, and modified slightly to handle timezones of the form [-+]DDDD, + * and also to fix a bug in the application of time zone to the parsed date. + * + * Date parser for ISO 8601 format + * http://www.w3.org/TR/1998/NOTE-datetime-19980827 + * @version $Revision: 1.3 $ + * @author bmahe@w3.org + */ + +public class DateParserTest { + + private static void test(String isodate) { + System.out.println("----------------------------------"); + try { + Date date = DateParser.parse(isodate); + System.out.println(">> " + isodate); + System.out.println(">> " + date.toString() + " [" + date.getTime() + "]"); + System.out.println(">> " + DateParser.getIsoDate(date)); + } catch (DateParser.InvalidDateException ex) { + System.err.println(isodate + " is invalid"); + System.err.println(ex.getMessage()); + } + System.out.println("----------------------------------"); + } + + private static void test(Date date) { + String isodate = null; + System.out.println("----------------------------------"); + try { + System.out.println(">> " + date.toString() + " [" + date.getTime() + "]"); + isodate = DateParser.getIsoDate(date); + System.out.println(">> " + isodate); + date = DateParser.parse(isodate); + System.out.println(">> " + date.toString() + " [" + date.getTime() + "]"); + } catch (DateParser.InvalidDateException ex) { + System.err.println(isodate + " is invalid"); + System.err.println(ex.getMessage()); + } + System.out.println("----------------------------------"); + } + + @DataProvider(name="dateDate") + public Object[][] dateData() { + return new Object[][]{ + {"1997-07-16T19:20:30.45-02:00"}, + {"1997-07-16T19:20:30+01:00"}, + {"1997-07-16T19:20:30+01:00"}, + {"1997-07-16T19:20"}, + {"1997-07-16"}, + {"1997-07"}, + {"1997"}, + }; + } + + @Test(dataProvider = "dateDate") + public static void testString(final String string) { + test(string); + } + + @Test + public static void testDate() { + test(new Date()); + } + + +} \ No newline at end of file diff --git a/src/test/java/htsjdk/samtools/util/IntervalTreeMapTest.java b/src/test/java/htsjdk/samtools/util/IntervalTreeMapTest.java index 2e725ff43..533f9652e 100644 --- a/src/test/java/htsjdk/samtools/util/IntervalTreeMapTest.java +++ b/src/test/java/htsjdk/samtools/util/IntervalTreeMapTest.java @@ -37,8 +37,8 @@ public void testBasic() { m.put(chr1Interval, chr1Interval); Interval chr2Interval = new Interval("chr2", 1,200); m.put(chr2Interval, chr2Interval); - - + + Assert.assertTrue(m.containsContained(new Interval("chr1", 9,101))); Assert.assertTrue(m.containsOverlapping(new Interval("chr1", 50,150))); Assert.assertFalse(m.containsOverlapping(new Interval("chr3", 1,100))); diff --git a/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java b/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java index 50d84c0b9..d50df4e6f 100644 --- a/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java +++ b/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java @@ -24,10 +24,13 @@ package htsjdk.samtools.util; import org.testng.Assert; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import java.util.Iterator; +import static htsjdk.samtools.util.IntervalTree.Node.HAS_OVERLAPPING_PART; + /** * @author alecw@broadinstitute.org */ @@ -57,10 +60,12 @@ private int countElements(final Iterator> it) { return ret; } - @Test - public void testMatches() - { - final IntervalTree intervalTree = new IntervalTree(); + private final IntervalTree intervalTree = new IntervalTree(); + + @BeforeMethod + public void init(){ //due to the destructive nature of removeMany test... + intervalTree.clear(); + intervalTree.put(1, 10, "foo1"); intervalTree.put(2, 9, "foo2"); intervalTree.put(3, 8, "foo3"); @@ -68,6 +73,87 @@ public void testMatches() intervalTree.put(5, 6, "foo5"); intervalTree.put(1, 9, "foo6"); + } + @Test + public void testRank() { + for (IntervalTree.Node node: intervalTree) { + Assert.assertEquals(intervalTree.findByIndex( + intervalTree.getIndex(node.getStart(), node.getEnd())), node); + } + } + + @Test + public void testIterator() { + + final IntervalTree.Node testNode = new IntervalTree.Node<>(3, 4, "foobar1"); + int count = 0; + Iterator> iterator = intervalTree.iterator(testNode.getStart(), testNode.getEnd()); + Iterable> iterable = () -> iterator; + for (IntervalTree.Node node : iterable) { + Assert.assertTrue(node.compare(testNode.getStart(), testNode.getEnd()) <= 0); + count++; + } + Assert.assertEquals(count, 3); // foobar3, foobar4, and foobar5 only. + } + + @Test + public void testRemoveMany() { + Iterator> iterator = intervalTree.reverseIterator(); + Iterable> iterable = () -> iterator; + + for (IntervalTree.Node node : iterable) { + intervalTree.removeNode(node); + } + Assert.assertEquals(intervalTree.size(), 0); + } + + @Test + public void testRevIterator() { + + final IntervalTree.Node testNode = new IntervalTree.Node<>(3, 4, "foobar1"); + int count = 0; + Iterator> iterator = intervalTree.reverseIterator(testNode.getStart(), testNode.getEnd()); + Iterable> iterable = () -> iterator; + for (IntervalTree.Node node : iterable) { + Assert.assertTrue(node.compare(testNode.getStart(), testNode.getEnd()) >= 0); + count++; + } + Assert.assertEquals(count, 3); // foobar1, foobar2, and foobar6 + } + + + @Test + public void testOverlapIterator() { + + final IntervalTree.Node testNode = new IntervalTree.Node<>(3, 4, "foobar1"); + int count = 0; + Iterator> iterator = intervalTree.overlappers(testNode.getStart(), testNode.getEnd()); + Iterable> iterable = () -> iterator; + for (IntervalTree.Node node : iterable) { + Assert.assertTrue( (testNode.getRelationship(node) & HAS_OVERLAPPING_PART) != 0, String.format("%s with %s = %d", node.toString(), testNode.toString(), node.getRelationship(testNode))); + count++; + } + Assert.assertEquals(count, 5); // foobar1, foobar2, foobar3, foobar4, and foobar6 + } + + + @Test + public void testTotalRevIterator() { + + int count = 0; + Iterator> iterator = intervalTree.reverseIterator(); + Iterable> iterable = () -> iterator; + + for (IntervalTree.Node ignored : iterable) { + count++; + } + Assert.assertEquals(count, intervalTree.size()); // foobar1, foobar2, and foobar6 + } + + + @Test + public void testMatches() + { // Single match Assert.assertEquals(countElements(intervalTree.overlappers(10, 10)), 1, "Test single overlap"); Assert.assertTrue(iteratorContains(intervalTree.overlappers(10, 10), "foo1"), "Test single overlap for correct overlapee"); diff --git a/src/test/java/htsjdk/samtools/util/SequenceUtilTest.java b/src/test/java/htsjdk/samtools/util/SequenceUtilTest.java index 008cca507..06df1f5d5 100644 --- a/src/test/java/htsjdk/samtools/util/SequenceUtilTest.java +++ b/src/test/java/htsjdk/samtools/util/SequenceUtilTest.java @@ -27,14 +27,13 @@ import htsjdk.samtools.reference.ReferenceSequence; import htsjdk.samtools.reference.ReferenceSequenceFile; import htsjdk.samtools.reference.ReferenceSequenceFileFactory; +import htsjdk.tribble.TestUtils; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.io.File; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; +import java.util.*; /** * @author alecw@broadinstitute.org @@ -144,6 +143,12 @@ public void testCountMismatches(final String readString, final String cigar, fin final SAMRecord rec = new SAMRecord(null); rec.setReadName("test"); rec.setReadString(readString); + final byte[] byteArray = new byte[readString.length()]; + for(int i =0; i< readString.length();i++){ + byteArray[i]=(byte)33; + } + + rec.setBaseQualities(byteArray); rec.setCigarString(cigar); final byte[] refBases = StringUtil.stringToBytes(reference); @@ -151,6 +156,9 @@ public void testCountMismatches(final String readString, final String cigar, fin final int nExact = SequenceUtil.countMismatches(rec, refBases, -1, false, false); Assert.assertEquals(nExact, expectedMismatchesExact); + final int sumMismatchesQualityExact = SequenceUtil.sumQualitiesOfMismatches(rec, refBases, -1, false); + Assert.assertEquals(sumMismatchesQualityExact, expectedMismatchesExact * 33); + final int nAmbiguous = SequenceUtil.countMismatches(rec, refBases, -1, false, true); Assert.assertEquals(nAmbiguous, expectedMismatchesAmbiguous); } @@ -175,6 +183,76 @@ public void testCountMismatches(final String readString, final String cigar, fin }; } + @DataProvider(name="mismatchBisulfiteCountsDataProvider") + public Object[][] mismatchBisulfiteCountsDataProvider() { + + return new Object[][] { + + {"A", "1M", "A", true, 0}, + {"A", "1M", "A", false, 0}, + {"A", "1M", "C", true, 1}, + {"A", "1M", "C", false, 1}, + {"A", "1M", "T", true, 1}, + {"A", "1M", "T", false, 1}, + {"A", "1M", "G", true, 1}, + {"A", "1M", "G", false, 0}, // bisulfite + + {"C", "1M", "A", true, 1}, + {"C", "1M", "A", false, 1}, + {"C", "1M", "C", true, 0}, + {"C", "1M", "C", false, 0}, + {"C", "1M", "T", true, 1}, + {"C", "1M", "T", false, 1}, + {"C", "1M", "G", true, 1}, + {"C", "1M", "G", false, 1}, + + {"T", "1M", "A", true, 1}, + {"T", "1M", "A", false, 1}, + {"T", "1M", "C", true, 0}, // bisulfite + {"T", "1M", "C", false, 1}, + {"T", "1M", "T", true, 0}, + {"T", "1M", "T", false, 0}, + {"T", "1M", "G", true, 1}, + {"T", "1M", "G", false, 1}, + + {"G", "1M", "A", true, 1}, + {"G", "1M", "A", false, 1}, + {"G", "1M", "C", true, 1}, + {"G", "1M", "C", false, 1}, + {"G", "1M", "T", true, 1}, + {"G", "1M", "T", false, 1}, + {"G", "1M", "G", true, 0}, + {"G", "1M", "G", false, 0} + }; + } + + + @Test(dataProvider = "mismatchBisulfiteCountsDataProvider") + public void testMismatchBisulfiteCounts(final String readString, final String cigar, final String reference, + final boolean positiveStrand, final int expectedMismatches) { + + final byte baseQuality = 30; + final SAMRecord rec = new SAMRecord(null); + rec.setReadName("test"); + rec.setReadString(readString); + rec.setReadNegativeStrandFlag(!positiveStrand); + final byte[] byteArray = new byte[readString.length()]; + for(int i = 0; i < readString.length();i++){ + byteArray[i]=baseQuality; + } + rec.setBaseQualities(byteArray); + rec.setCigarString(cigar); + + final byte[] refBases = StringUtil.stringToBytes(reference); + + final int nExact = SequenceUtil.countMismatches(rec, refBases, -1, true, false); + Assert.assertEquals(nExact, expectedMismatches); + + final int sumMismatchesQualityExact = SequenceUtil.sumQualitiesOfMismatches(rec, refBases, -1, true); + Assert.assertEquals(sumMismatchesQualityExact, expectedMismatches * baseQuality); + + } + @Test(dataProvider = "countInsertedAndDeletedBasesTestCases") public void testCountInsertedAndDeletedBases(final String cigarString, final int insertedBases, final int deletedBases) { final Cigar cigar = TextCigarCodec.decode(cigarString); @@ -450,4 +528,58 @@ public void testCalculateNmTag() { } }); } + + @DataProvider(name = "testNmFromCigarProvider") + Object[][] testNmFromCigar() { + return new Object[][]{ + {"1M", 0}, + {"1S1D", 1}, + {"1H3X", 3}, + {"1H5=3M2X", 2}, + {"5P5M", 0}, + {"5S8I", 8} + }; + } + + @Test(dataProvider = "testNmFromCigarProvider") + public void testNmTagFromCigar(final String cigarString, final int expectedNmValue) { + final SAMRecord rec = new SAMRecord(null); + rec.setReadName("test"); + rec.setCigarString(cigarString); + + Assert.assertEquals(SequenceUtil.calculateSamNmTagFromCigar(rec),expectedNmValue); + } + + @Test + public void testReverseComplement() { + Assert.assertEquals(SequenceUtil.reverseComplement("ABCDEFGHIJKLMNOPQRSTUVWXYZ"),"ZYXWVUASRQPONMLKJIHCFEDGBT"); + Assert.assertEquals(SequenceUtil.reverseComplement("abcdefghijklmnopqrstuvwxy"),"yxwvuasrqponmlkjihcfedgbt"); //missing "z" on purpose so that we test both even-lengthed and odd-lengthed strings + } + + @Test + public void testUpperCase() { + Assert.assertEquals(SequenceUtil.upperCase(StringUtil.stringToBytes("ABCDEFGHIJKLMNOPQRSTUVWXYZ")), StringUtil.stringToBytes("ABCDEFGHIJKLMNOPQRSTUVWXYZ")); + Assert.assertEquals(SequenceUtil.upperCase(StringUtil.stringToBytes("abcdefghijklmnopqrstuvwxyz")), StringUtil.stringToBytes("ABCDEFGHIJKLMNOPQRSTUVWXYZ")); + Assert.assertEquals(SequenceUtil.upperCase(StringUtil.stringToBytes("1234567890!@#$%^&*()")), StringUtil.stringToBytes("1234567890!@#$%^&*()")); + } + + @Test + public void testReverseQualities() { + + final byte[] qualities1 = new byte[] {10, 20, 30, 40}; + SequenceUtil.reverseQualities(qualities1); + assertEquals(qualities1, new byte[] {40, 30, 20, 10}); + + final byte[] qualities2 = {10, 20, 30}; + SequenceUtil.reverseQualities(qualities2); + assertEquals(qualities2, new byte[]{30, 20, 10}); + } + + private void assertEquals(final byte[] actual, final byte[] expected) { + Assert.assertEquals(actual.length, expected.length, "Arrays do not have equal lengths"); + + for (int i = 0; i < actual.length; ++i) { + Assert.assertEquals(actual[i], expected[i], "Array differ at position " + i); + } + } } From 87a647a0e4fe0b888c6ad1fe9cd5ef27e52ad805 Mon Sep 17 00:00:00 2001 From: Yossi Farjoun Date: Sat, 7 Jan 2017 14:48:12 -0500 Subject: [PATCH 2/4] - responding to reviewer's comments. --- .../java/htsjdk/samtools/util/IntervalTree.java | 4 +- .../java/htsjdk/samtools/util/DateParserTest.java | 37 ++++++++++-- .../htsjdk/samtools/util/IntervalTreeTest.java | 53 +++++++++++++++-- .../htsjdk/samtools/util/SequenceUtilTest.java | 69 ++++++++-------------- 4 files changed, 105 insertions(+), 58 deletions(-) diff --git a/src/main/java/htsjdk/samtools/util/IntervalTree.java b/src/main/java/htsjdk/samtools/util/IntervalTree.java index be4ae130e..cd4acdb69 100644 --- a/src/main/java/htsjdk/samtools/util/IntervalTree.java +++ b/src/main/java/htsjdk/samtools/util/IntervalTree.java @@ -482,7 +482,7 @@ public int getEnd() public int getLength() { - return mEnd - mStart; + return mEnd - mStart + 1 ; } public int getRelationship( final Node interval ) @@ -499,7 +499,7 @@ public int getRelationship( final Node interval ) public boolean isAdjacent( final Node interval ) { - return mStart == interval.getEnd() || mEnd == interval.getStart(); + return mStart == interval.getEnd() + 1 || mEnd + 1 == interval.getStart(); } public V1 getValue() diff --git a/src/test/java/htsjdk/samtools/util/DateParserTest.java b/src/test/java/htsjdk/samtools/util/DateParserTest.java index 4a28cc6d6..c26158429 100644 --- a/src/test/java/htsjdk/samtools/util/DateParserTest.java +++ b/src/test/java/htsjdk/samtools/util/DateParserTest.java @@ -72,9 +72,11 @@ This W3C work (including software, documents, or other related items) is package htsjdk.samtools.util; +import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import java.text.DateFormat; import java.util.Date; /** @@ -89,13 +91,20 @@ This W3C work (including software, documents, or other related items) is public class DateParserTest { - private static void test(String isodate) { + private static void test(final String isodate) { System.out.println("----------------------------------"); try { Date date = DateParser.parse(isodate); System.out.println(">> " + isodate); System.out.println(">> " + date.toString() + " [" + date.getTime() + "]"); - System.out.println(">> " + DateParser.getIsoDate(date)); + final String isodateRoundTrip = DateParser.getIsoDate(date); + System.out.println(">> " + isodateRoundTrip); + + final Date orig = DateParser.parse(isodate); + final Date roundTrip = DateParser.parse(isodateRoundTrip); + + assertDatesAreClose(orig, roundTrip); + } catch (DateParser.InvalidDateException ex) { System.err.println(isodate + " is invalid"); System.err.println(ex.getMessage()); @@ -103,15 +112,19 @@ private static void test(String isodate) { System.out.println("----------------------------------"); } - private static void test(Date date) { + private static void test(final Date date) { String isodate = null; System.out.println("----------------------------------"); try { System.out.println(">> " + date.toString() + " [" + date.getTime() + "]"); isodate = DateParser.getIsoDate(date); System.out.println(">> " + isodate); - date = DateParser.parse(isodate); + final Date dateRoundTrip = DateParser.parse(isodate); System.out.println(">> " + date.toString() + " [" + date.getTime() + "]"); + + assertDatesAreClose(date, dateRoundTrip); + Assert.assertTrue(Math.abs(Date.parse(date.toString())-Date.parse(dateRoundTrip.toString()))<10); + } catch (DateParser.InvalidDateException ex) { System.err.println(isodate + " is invalid"); System.err.println(ex.getMessage()); @@ -137,10 +150,24 @@ public static void testString(final String string) { test(string); } + @Test(dataProvider = "dateDate") + public static void testDates(final String string) { + test(DateParser.parse(string)); + } + @Test public static void testDate() { test(new Date()); } - + public static void assertDatesAreClose(final Date lhs, final Date rhs) { + Assert.assertEquals(lhs.getYear(), rhs.getYear()); + Assert.assertEquals(lhs.getMonth(), rhs.getMonth()); + Assert.assertEquals(lhs.getDate(), rhs.getDate()); + Assert.assertEquals(lhs.getDay(), rhs.getDay()); + Assert.assertEquals(lhs.getHours(), rhs.getHours()); + Assert.assertEquals(lhs.getMinutes(), rhs.getMinutes()); + Assert.assertEquals(lhs.getSeconds(), rhs.getSeconds()); + Assert.assertEquals(lhs.getTimezoneOffset(), rhs.getTimezoneOffset()); + } } \ No newline at end of file diff --git a/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java b/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java index d50df4e6f..a8109c7b7 100644 --- a/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java +++ b/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java @@ -25,6 +25,7 @@ import org.testng.Assert; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.util.Iterator; @@ -34,6 +35,7 @@ /** * @author alecw@broadinstitute.org */ +@Test(singleThreaded=true) // to assure that the common resources aren't clobbered public class IntervalTreeTest { @Test public void testNoMatches() @@ -66,14 +68,51 @@ private int countElements(final Iterator> it) { public void init(){ //due to the destructive nature of removeMany test... intervalTree.clear(); - intervalTree.put(1, 10, "foo1"); - intervalTree.put(2, 9, "foo2"); - intervalTree.put(3, 8, "foo3"); - intervalTree.put(4, 7, "foo4"); - intervalTree.put(5, 6, "foo5"); - intervalTree.put(1, 9, "foo6"); + // each interval has a "name:length" + intervalTree.put(1, 10, "foo1:10"); + intervalTree.put(2, 9, "foo2:8"); + intervalTree.put(3, 8, "foo3:6"); + intervalTree.put(4, 7, "foo4:4"); + intervalTree.put(5, 6, "foo5:2"); + intervalTree.put(1, 9, "foo6:9"); + } + + @Test + public void testLength(){ + + Iterator> iterator = intervalTree.iterator(); + Iterable> iterable = () -> iterator; + + for (IntervalTree.Node node : iterable) { + Assert.assertEquals(node.getLength(), Integer.parseInt(node.getValue().replaceAll(".*:", ""))); + } + } + + @DataProvider(name="adjacentIntervalsTestData") + public Object[][] adjacentIntervalsTestData() { + return new Object[][]{ + {1, 4, 5, 10, true}, + {1, 3, 5, 10, false}, + {1, 4, 6, 10, false}, + {1, 2, 6, 10, false}, + {1, 10, 6, 10, false}, + {1, 10, 11, 20, true}, + {1, 10, 11, 20, true}, + }; + } + + @Test(dataProvider = "adjacentIntervalsTestData") + public void testAdjacent(int start1, int end1, int start2, int end2, boolean areAdjacent){ + + final IntervalTree.Node node1 = new IntervalTree.Node<>(start1, end1, "one"); + final IntervalTree.Node node2 = new IntervalTree.Node<>(start2, end2, "two"); + + Assert.assertTrue(node1.isAdjacent(node2) == areAdjacent); + Assert.assertTrue(node2.isAdjacent(node1) == areAdjacent); } + + @Test public void testRank() { for (IntervalTree.Node node: intervalTree) { @@ -270,4 +309,6 @@ public void testRemove() { Assert.assertEquals(intervalTree.remove(46402360, 46402594), "frob"); intervalTree.checkMaxEnds(); } + + } diff --git a/src/test/java/htsjdk/samtools/util/SequenceUtilTest.java b/src/test/java/htsjdk/samtools/util/SequenceUtilTest.java index 06df1f5d5..7c96b742d 100644 --- a/src/test/java/htsjdk/samtools/util/SequenceUtilTest.java +++ b/src/test/java/htsjdk/samtools/util/SequenceUtilTest.java @@ -24,10 +24,8 @@ package htsjdk.samtools.util; import htsjdk.samtools.*; -import htsjdk.samtools.reference.ReferenceSequence; import htsjdk.samtools.reference.ReferenceSequenceFile; import htsjdk.samtools.reference.ReferenceSequenceFileFactory; -import htsjdk.tribble.TestUtils; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -144,9 +142,8 @@ public void testCountMismatches(final String readString, final String cigar, fin rec.setReadName("test"); rec.setReadString(readString); final byte[] byteArray = new byte[readString.length()]; - for(int i =0; i< readString.length();i++){ - byteArray[i]=(byte)33; - } + + Arrays.fill(byteArray, (byte)33); rec.setBaseQualities(byteArray); rec.setCigarString(cigar); @@ -186,44 +183,26 @@ public void testCountMismatches(final String readString, final String cigar, fin @DataProvider(name="mismatchBisulfiteCountsDataProvider") public Object[][] mismatchBisulfiteCountsDataProvider() { - return new Object[][] { + List tests = new ArrayList<>(); + final List bases = Arrays.asList("A","C","T","G"); - {"A", "1M", "A", true, 0}, - {"A", "1M", "A", false, 0}, - {"A", "1M", "C", true, 1}, - {"A", "1M", "C", false, 1}, - {"A", "1M", "T", true, 1}, - {"A", "1M", "T", false, 1}, - {"A", "1M", "G", true, 1}, - {"A", "1M", "G", false, 0}, // bisulfite - - {"C", "1M", "A", true, 1}, - {"C", "1M", "A", false, 1}, - {"C", "1M", "C", true, 0}, - {"C", "1M", "C", false, 0}, - {"C", "1M", "T", true, 1}, - {"C", "1M", "T", false, 1}, - {"C", "1M", "G", true, 1}, - {"C", "1M", "G", false, 1}, - - {"T", "1M", "A", true, 1}, - {"T", "1M", "A", false, 1}, - {"T", "1M", "C", true, 0}, // bisulfite - {"T", "1M", "C", false, 1}, - {"T", "1M", "T", true, 0}, - {"T", "1M", "T", false, 0}, - {"T", "1M", "G", true, 1}, - {"T", "1M", "G", false, 1}, - - {"G", "1M", "A", true, 1}, - {"G", "1M", "A", false, 1}, - {"G", "1M", "C", true, 1}, - {"G", "1M", "C", false, 1}, - {"G", "1M", "T", true, 1}, - {"G", "1M", "T", false, 1}, - {"G", "1M", "G", true, 0}, - {"G", "1M", "G", false, 0} - }; + for (final String base : bases) { + for (final String ref : bases) { + for (final Boolean strand : Arrays.asList(true, false)) { + + final Integer count; + + if (base.equals(ref)) count = 0; + else if (base.equals("A") && ref.equals("G") && !strand) count = 0; + else if (base.equals("T") && ref.equals("C") && strand) count = 0; + else count = 1; + + tests.add(new Object[]{base, "1M", ref, strand, count}); + + } + } + } + return tests.toArray(new Object[1][]); } @@ -237,9 +216,9 @@ public void testMismatchBisulfiteCounts(final String readString, final String ci rec.setReadString(readString); rec.setReadNegativeStrandFlag(!positiveStrand); final byte[] byteArray = new byte[readString.length()]; - for(int i = 0; i < readString.length();i++){ - byteArray[i]=baseQuality; - } + + Arrays.fill(byteArray,baseQuality); + rec.setBaseQualities(byteArray); rec.setCigarString(cigar); From c7988191a30c8f29d3afc3d83e9c34c833affeb8 Mon Sep 17 00:00:00 2001 From: Yossi Farjoun Date: Sat, 7 Jan 2017 17:12:47 -0500 Subject: [PATCH 3/4] -- oops, I broke a test...fixed. --- src/test/java/htsjdk/samtools/util/IntervalTreeTest.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java b/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java index a8109c7b7..34cb3c5be 100644 --- a/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java +++ b/src/test/java/htsjdk/samtools/util/IntervalTreeTest.java @@ -88,7 +88,6 @@ public void testLength(){ } } - @DataProvider(name="adjacentIntervalsTestData") public Object[][] adjacentIntervalsTestData() { return new Object[][]{ @@ -189,22 +188,21 @@ public void testTotalRevIterator() { Assert.assertEquals(count, intervalTree.size()); // foobar1, foobar2, and foobar6 } - @Test public void testMatches() { // Single match Assert.assertEquals(countElements(intervalTree.overlappers(10, 10)), 1, "Test single overlap"); - Assert.assertTrue(iteratorContains(intervalTree.overlappers(10, 10), "foo1"), "Test single overlap for correct overlapee"); + Assert.assertTrue(iteratorContains(intervalTree.overlappers(10, 10), "foo1:10"), "Test single overlap for correct overlapee"); // Multiple matches Assert.assertEquals(countElements(intervalTree.overlappers(7, 8)), 5, "Test multiple overlap"); - Assert.assertTrue(iteratorContains(intervalTree.overlappers(7, 8), "foo1"), "Test multiple overlap for correct overlapees"); - Assert.assertTrue(iteratorContains(intervalTree.overlappers(7, 8), "foo2"), "Test multiple overlap for correct overlapees"); - Assert.assertTrue(iteratorContains(intervalTree.overlappers(7, 8), "foo3"), "Test multiple overlap for correct overlapees"); - Assert.assertTrue(iteratorContains(intervalTree.overlappers(7, 8), "foo4"), "Test multiple overlap for correct overlapees"); - Assert.assertTrue(iteratorContains(intervalTree.overlappers(7, 8), "foo6"), "Test multiple overlap for correct overlapees"); - Assert.assertTrue(!iteratorContains(intervalTree.overlappers(7, 8), "foo5"), "Test multiple overlap for correct overlapees"); + Assert.assertTrue( iteratorContains(intervalTree.overlappers(7, 8), "foo1:10"), "Test multiple overlap for correct overlapees"); + Assert.assertTrue( iteratorContains(intervalTree.overlappers(7, 8), "foo2:8"), "Test multiple overlap for correct overlapees"); + Assert.assertTrue( iteratorContains(intervalTree.overlappers(7, 8), "foo3:6"), "Test multiple overlap for correct overlapees"); + Assert.assertTrue( iteratorContains(intervalTree.overlappers(7, 8), "foo4:4"), "Test multiple overlap for correct overlapees"); + Assert.assertTrue( iteratorContains(intervalTree.overlappers(7, 8), "foo6:9"), "Test multiple overlap for correct overlapees"); + Assert.assertTrue(!iteratorContains(intervalTree.overlappers(7, 8), "foo5:2"), "Test multiple overlap for correct overlapees"); } private boolean iteratorContains(final Iterator> nodeIterator, final String s) { From 48c6985ca4297891d2858b5251c4ed0c00cd6ba4 Mon Sep 17 00:00:00 2001 From: Yossi Farjoun Date: Fri, 20 Jan 2017 00:53:11 -0500 Subject: [PATCH 4/4] - responding to second round of review --- .../java/htsjdk/samtools/util/DateParserTest.java | 49 +++++++--------------- 1 file changed, 14 insertions(+), 35 deletions(-) diff --git a/src/test/java/htsjdk/samtools/util/DateParserTest.java b/src/test/java/htsjdk/samtools/util/DateParserTest.java index c26158429..71313c15b 100644 --- a/src/test/java/htsjdk/samtools/util/DateParserTest.java +++ b/src/test/java/htsjdk/samtools/util/DateParserTest.java @@ -92,44 +92,23 @@ This W3C work (including software, documents, or other related items) is public class DateParserTest { private static void test(final String isodate) { - System.out.println("----------------------------------"); - try { - Date date = DateParser.parse(isodate); - System.out.println(">> " + isodate); - System.out.println(">> " + date.toString() + " [" + date.getTime() + "]"); - final String isodateRoundTrip = DateParser.getIsoDate(date); - System.out.println(">> " + isodateRoundTrip); - - final Date orig = DateParser.parse(isodate); - final Date roundTrip = DateParser.parse(isodateRoundTrip); - - assertDatesAreClose(orig, roundTrip); - - } catch (DateParser.InvalidDateException ex) { - System.err.println(isodate + " is invalid"); - System.err.println(ex.getMessage()); - } - System.out.println("----------------------------------"); + Date date = DateParser.parse(isodate); + final String isodateRoundTrip = DateParser.getIsoDate(date); + + final Date orig = DateParser.parse(isodate); + final Date roundTrip = DateParser.parse(isodateRoundTrip); + + assertDatesAreClose(orig, roundTrip); } private static void test(final Date date) { - String isodate = null; - System.out.println("----------------------------------"); - try { - System.out.println(">> " + date.toString() + " [" + date.getTime() + "]"); - isodate = DateParser.getIsoDate(date); - System.out.println(">> " + isodate); - final Date dateRoundTrip = DateParser.parse(isodate); - System.out.println(">> " + date.toString() + " [" + date.getTime() + "]"); - - assertDatesAreClose(date, dateRoundTrip); - Assert.assertTrue(Math.abs(Date.parse(date.toString())-Date.parse(dateRoundTrip.toString()))<10); - - } catch (DateParser.InvalidDateException ex) { - System.err.println(isodate + " is invalid"); - System.err.println(ex.getMessage()); - } - System.out.println("----------------------------------"); + String isodate; + isodate = DateParser.getIsoDate(date); + final Date dateRoundTrip = DateParser.parse(isodate); + + assertDatesAreClose(date, dateRoundTrip); + Assert.assertTrue(Math.abs(Date.parse(date.toString()) - Date.parse(dateRoundTrip.toString())) < 10); + } @DataProvider(name="dateDate")