Increase Test Coverage, fix tiny bug in Node class (regarding OVERLAPS) #779

Merged
merged 4 commits into from Jan 20, 2017
@@ -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() {
}
@@ -482,7 +482,7 @@ public int getEnd()
public int getLength()
{
- return mEnd - mStart;
+ return mEnd - mStart + 1 ;
}
public int getRelationship( final Node<V1> interval )
@@ -492,14 +492,14 @@ public int getRelationship( final Node<V1> 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 )
@lbergelson

lbergelson Jan 6, 2017

Contributor

Good catch. I'm surprised we haven't noticed this bug when using OverlapDetector...

@lbergelson

lbergelson Jan 6, 2017

Contributor

I think there's a similar bug in getLength(). Shouldn't it be end - start +1?

Also in isAdjacent(), it needs to adjust by 1 on both ends I think...

@yfarjoun

yfarjoun Jan 7, 2017

Contributor

thankfully all of these members are unused, which is why it went undetected.....the getLength function has no contract...so it's not clear what the intention was...but sure, I'll change it.

result |= HAS_OVERLAPPING_PART;
return result;
}
public boolean isAdjacent( final Node<V1> interval )
{
- return mStart == interval.getEnd() || mEnd == interval.getStart();
+ return mStart == interval.getEnd() + 1 || mEnd + 1 == interval.getStart();
}
public V1 getValue()
@@ -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);
@lbergelson

lbergelson Jan 6, 2017

Contributor

👍 for code reuse

+ }
+
+ /** 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;
@@ -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<SAMRecord> it = reader.query("chr1", 202661637, 202661812, contained);
+ Assert.assertEquals(countElements(it), expected);
+ }
+
@Test
public void testQueryMate() {
final SamReader reader = SamReaderFactory.makeDefault().open(BAM_FILE);
@@ -0,0 +1,152 @@
+// 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.Assert;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import java.text.DateFormat;
+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(final String isodate) {
+ 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;
+ 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")
+ 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) {
@lbergelson

lbergelson Jan 6, 2017

Contributor

What are these tests supposed to be accomplishing? They execute the date parsing code, but they don't actually assert anything.

On a side note, is the date parser something we should continue to support? I'm guessing it exists to deal with issues in java.util.Date and lack of functionality there. Now that we have java.util.time we may be able to deprecate and replace it.

@yfarjoun

yfarjoun Jan 7, 2017

Contributor

out of scope.

+ 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());
+ }
+}
@@ -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)));
Oops, something went wrong.