From 06dd1c1a5244aaf7a41437326870d2be7fc5aea7 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Fri, 5 Oct 2018 17:11:17 -0400 Subject: [PATCH 01/20] Adding support for 0-length B arrays in SAM files to conform to 1.6 spec. --- .../java/htsjdk/samtools/SAMLineParser.java | 5 +- src/main/java/htsjdk/samtools/SAMRecord.java | 3 - .../java/htsjdk/samtools/TextTagCodec.java | 26 +- .../htsjdk/samtools/SAMRecordUnitTest.java | 93 +++++- .../htsjdk/samtools/one-contig.fasta | 278 ++++++++++++++++++ 5 files changed, 391 insertions(+), 14 deletions(-) create mode 100644 src/test/resources/htsjdk/samtools/one-contig.fasta diff --git a/src/main/java/htsjdk/samtools/SAMLineParser.java b/src/main/java/htsjdk/samtools/SAMLineParser.java index f73a67e08e..c015aa0a5a 100644 --- a/src/main/java/htsjdk/samtools/SAMLineParser.java +++ b/src/main/java/htsjdk/samtools/SAMLineParser.java @@ -465,10 +465,9 @@ private void reportErrorParsingLine(final String reason) { private void reportErrorParsingLine(final Exception e) { final String errorMessage = makeErrorString(e.getMessage()); if (validationStringency == ValidationStringency.STRICT) { - throw new SAMFormatException(errorMessage); + throw new SAMFormatException(errorMessage, e); } else if (validationStringency == ValidationStringency.LENIENT) { - System.err - .println("Ignoring SAM validation error due to lenient parsing:"); + System.err.println("Ignoring SAM validation error due to lenient parsing:"); System.err.println(errorMessage); } } diff --git a/src/main/java/htsjdk/samtools/SAMRecord.java b/src/main/java/htsjdk/samtools/SAMRecord.java index 3146f6b48a..c5bcbdffb1 100644 --- a/src/main/java/htsjdk/samtools/SAMRecord.java +++ b/src/main/java/htsjdk/samtools/SAMRecord.java @@ -1419,9 +1419,6 @@ public Object getAttribute(final short tag) { * String values are not validated to ensure that they conform to SAM spec. */ public void setAttribute(final String tag, final Object value) { - if (value != null && value.getClass().isArray() && Array.getLength(value) == 0) { - throw new IllegalArgumentException("Empty value passed for tag " + tag); - } setAttribute(SAMTagUtil.getSingleton().makeBinaryTag(tag), value); } diff --git a/src/main/java/htsjdk/samtools/TextTagCodec.java b/src/main/java/htsjdk/samtools/TextTagCodec.java index 40dc8ac73f..6d38761106 100644 --- a/src/main/java/htsjdk/samtools/TextTagCodec.java +++ b/src/main/java/htsjdk/samtools/TextTagCodec.java @@ -98,8 +98,11 @@ private char getArrayType(final Object array, final boolean isUnsigned) { } private String encodeArrayValue(final Object value) { - final StringBuilder ret = new StringBuilder(Array.get(value, 0).toString()); final int length = Array.getLength(value); + if(length == 0){ + return ""; + } + final StringBuilder ret = new StringBuilder(Array.get(value, 0).toString()); for (int i = 1; i < length; ++i) { ret.append(','); ret.append(Array.get(value, i).toString()); @@ -221,13 +224,20 @@ else if (SAMUtils.isValidUnsignedIntegerAttribute(lValue)) { private Object covertStringArrayToObject(final String stringVal) { final String[] elementTypeAndValue = new String[2]; - if (StringUtil.splitConcatenateExcessTokens(stringVal, elementTypeAndValue, ',') != 2) { - throw new SAMFormatException("Tag of type B should have an element type followed by comma"); + + final int numberOfTokens = StringUtil.splitConcatenateExcessTokens(stringVal, elementTypeAndValue, ','); + if (!(numberOfTokens == 1 || numberOfTokens == 2)) { + throw new SAMFormatException("Tag of type B requires an element type"); } if (elementTypeAndValue[0].length() != 1) { throw new SAMFormatException("Unrecognized element type for array tag value: " + elementTypeAndValue[0]); } + final char elementType = elementTypeAndValue[0].charAt(0); + if (numberOfTokens == 1) { + return createEmptyArray(elementType); + } + final String[] stringValues = elementTypeAndValue[1].split(","); if (stringValues.length == 0) throw new SAMFormatException("Tag of type B should have at least one element"); if (elementType == 'f') { @@ -316,6 +326,16 @@ private Object covertStringArrayToObject(final String stringVal) { } } + private static Object createEmptyArray(char elementType) { + switch( Character.toLowerCase(elementType) ) { + case 'c': { return new byte[0]; } + case 's': { return new short[0]; } + case 'i': { return new int[0]; } + case 'f': { return new float[0]; } + default: { throw new SAMFormatException("Unrecognized array tag element type: " + elementType); } + } + } + Iso8601Date decodeDate(final String dateStr) { try { return new Iso8601Date(dateStr); diff --git a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java index 135d5a321d..29de84229b 100644 --- a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java +++ b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java @@ -25,18 +25,19 @@ package htsjdk.samtools; import htsjdk.HtsjdkTest; +import htsjdk.samtools.cram.build.CramIO; import htsjdk.samtools.util.BinaryCodec; +import htsjdk.samtools.util.IOUtil; import htsjdk.samtools.util.TestUtil; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.io.*; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Optional; +import java.lang.reflect.Array; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.*; public class SAMRecordUnitTest extends HtsjdkTest { @@ -1169,4 +1170,86 @@ private Object[][] hasAttributeTestData() throws IOException { public void testHasAttribute(final SAMRecord samRecord, final String tag, final boolean expectedHasAttribute) { Assert.assertEquals(samRecord.hasAttribute(tag), expectedHasAttribute); } + + @Test + public void test_setAttribute_empty_array() { + final SAMFileHeader header = new SAMFileHeader(); + final String arrayTag = "XA"; + final SAMRecord record = new SAMRecord(header); + Assert.assertNull(record.getStringAttribute(arrayTag)); + record.setAttribute(arrayTag, new int[0]); + Assert.assertNotNull(record.getSignedIntArrayAttribute(arrayTag)); + Assert.assertEquals(record.getSignedIntArrayAttribute(arrayTag),new int[0]); + record.getSAMString(); + Assert.assertEquals(record.getAttribute(arrayTag), new char[0]); + record.setAttribute(arrayTag, null); + Assert.assertNull(record.getStringAttribute(arrayTag)); + + } + + @DataProvider + public Object[][] getEmptyArrays(){ + final String BAM = BamFileIoUtils.BAM_FILE_EXTENSION; + final String SAM = IOUtil.SAM_FILE_EXTENSION; + final String CRAM = CramIO.CRAM_FILE_EXTENSION; + return new Object[][]{ + {new int[0], int[].class, BAM}, + {new short[0], short[].class, BAM}, + {new byte[0], byte[].class, BAM}, + {new float[0], float[].class, BAM}, + + {new int[0], int[].class, SAM}, + {new short[0], short[].class, SAM}, + {new byte[0], byte[].class, SAM}, + {new float[0], float[].class, SAM}, + + {new int[0], int[].class, CRAM}, + {new short[0], short[].class, CRAM}, + {new byte[0], byte[].class, CRAM}, + {new float[0], float[].class, CRAM}, + + }; + } + + @Test(dataProvider = "getEmptyArrays") + public void testWriteSamWithEmptyArray(Object emptyArray, Class arrayClass, String fileExtension) throws IOException { + Assert.assertEquals(emptyArray.getClass(), arrayClass); + Assert.assertEquals(Array.getLength(emptyArray), 0); + + final String arrayTag = "XA"; + final SAMRecordSetBuilder samRecords = new SAMRecordSetBuilder(); + samRecords.addFrag("Read",0,100, false); + final SAMRecord record = samRecords.getRecords().iterator().next(); + record.setAttribute(arrayTag, emptyArray); + checkArrayIsEmpty(arrayTag, record, arrayClass); + + final Path tmp = Files.createTempFile("tmp", fileExtension); + IOUtil.deleteOnExit(tmp); + + final SAMFileWriterFactory writerFactory = new SAMFileWriterFactory() + .setCreateMd5File(false) + .setCreateIndex(false); + final Path reference = IOUtil.getPath("src/test/resources/htsjdk/samtools/one-contig.fasta"); + try(final SAMFileWriter samFileWriter = writerFactory.makeWriter(samRecords.getHeader(), false, tmp, reference)) { + samFileWriter.addAlignment(record); + } + + try(final SamReader reader = SamReaderFactory.makeDefault() + .referenceSequence(reference) + .open(tmp)) { + final SAMRecordIterator iterator = reader.iterator(); + Assert.assertTrue(iterator.hasNext()); + final SAMRecord recordFromDisk = iterator.next(); + checkArrayIsEmpty(arrayTag, recordFromDisk, arrayClass); + } + } + + private static void checkArrayIsEmpty(String arrayTag, SAMRecord recordFromDisk, Class expectedClass) { + final Object attribute = recordFromDisk.getAttribute(arrayTag); + Assert.assertNotNull(attribute); + Assert.assertEquals(attribute.getClass(), expectedClass); + Assert.assertEquals(Array.getLength(attribute), 0); + } + + } diff --git a/src/test/resources/htsjdk/samtools/one-contig.fasta b/src/test/resources/htsjdk/samtools/one-contig.fasta new file mode 100644 index 0000000000..8af5b5bf26 --- /dev/null +++ b/src/test/resources/htsjdk/samtools/one-contig.fasta @@ -0,0 +1,278 @@ +>chr1 +GATCACAGGTCTATCACCCTATTAACCACTCACGGGAGCTCTCCATGCATTTGGTATTTT +CGTCTGGGGGGTGTGCACGCGATAGCATTGCGAGACGCTGGAGCCGGAGCACCCTATGTC +GCAGTATCTGTCTTTGATTCCTGCCTCATTCTATTATTTATCGCACCTACGTTCAATATT +ACAGGCGAACATACCTACTAAAGTGTGTTAATTAATTAATGCTTGTAGGACATAATAATA +ACAATTGAATGTCTGCACAGCCGCTTTCCACACAGACATCATAACAAAAAATTTCCACCA +AACCCCCCCCTCCCCCCGCTTCTGGCCACAGCACTTAAACACATCTCTGCCAAACCCCAA +AAACAAAGAACCCTAACACCAGCCTAACCAGATTTCAAATTTTATCTTTAGGCGGTATGC +ACTTTTAACAGTCACCCCCCAACTAACACATTATTTTCCCCTCCCACTCCCATACTACTA +ATCTCATCAATACAACCCCCGCCCATCCTACCCAGCACACACACACCGCTGCTAACCCCA +TACCCCGAACCAACCAAACCCCAAAGACACCCCCCACAGTTTATGTAGCTTACCTCCTCA +AAGCAATACACTGAAAATGTTTAGACGGGCTCACATCACCCCATAAACAAATAGGTTTGG +TCCTAGCCTTTCTATTAGCTCTTAGTAAGATTACACATGCAAGCATCCCCGTTCCAGTGA +GTTCACCCTCTAAATCACCACGATCAAAAGGGACAAGCATCAAGCACGCAGCAATGCAGC +TCAAAACGCTTAGCCTAGCCACACCCCCACGGGAAACAGCAGTGATTAACCTTTAGCAAT +AAACGAAAGTTTAACTAAGCTATACTAACCCCAGGGTTGGTCAATTTCGTGCCAGCCACC +GCGGTCACACGATTAACCCAAGTCAATAGAAGCCGGCGTAAAGAGTGTTTTAGATCACCC +CCTCCCCAATAAAGCTAAAACTCACCTGAGTTGTAAAAAACTCCAGTTGACACAAAATAG +ACTACGAAAGTGGCTTTAACATATCTGAACACACAATAGCTAAGACCCAAACTGGGATTA +GATACCCCACTATGCTTAGCCCTAAACCTCAACAGTTAAATCAACAAAACTGCTCGCCAG +AACACTACGAGCCACAGCTTAAAACTCAAAGGACCTGGCGGTGCTTCATATCCCTCTAGA +GGAGCCTGTTCTGTAATCGATAAACCCCGATCAACCTCACCACCTCTTGCTCAGCCTATA +TACCGCCATCTTCAGCAAACCCTGATGAAGGCTACAAAGTAAGCGCAAGTACCCACGTAA +AGACGTTAGGTCAAGGTGTAGCCCATGAGGTGGCAAGAAATGGGCTACATTTTCTACCCC +AGAAAACTACGATAGCCCTTATGAAACTTAAGGGTCGAAGGTGGATTTAGCAGTAAACTG +AGAGTAGAGTGCTTAGTTGAACAGGGCCCTGAAGCGCGTACACACCGCCCGTCACCCTCC +TCAAGTATACTTCAAAGGACATTTAACTAAAACCCCTACGCATTTATATAGAGGAGACAA +GTCGTAACATGGTAAGTGTACTGGAAAGTGCACTTGGACGAACCAGAGTGTAGCTTAACA +CAAAGCACCCAACTTACACTTAGGAGATTTCAACTTAACTTGACCGCTCTGAGCTAAACC +TAGCCCCAAACCCACTCCACCTTACTACCAGACAACCTTAGCCAAACCATTTACCCAAAT +AAAGTATAGGCGATAGAAATTGAAACCTGGCGCAATAGATATAGTACCGCAAGGGAAAGA +TGAAAAATTATAACCAAGCATAATATAGCAAGGACTAACCCCTATACCTTCTGCATAATG +AATTAACTAGAAATAACTTTGCAAGGAGAGCCAAAGCTAAGACCCCCGAAACCAGACGAG +CTACCTAAGAACAGCTAAAAGAGCACACCCGTCTATGTAGCAAAATAGTGGGAAGATTTA +TAGGTAGAGGCGACAAACCTACCGAGCCTGGTGATAGCTGGTTGTCCAAGATAGAATCTT +AGTTCAACTTTAAATTTGCCCACAGAACCCTCTAAATCCCCTTGTAAATTTAACTGTTAG +TCCAAAGAGGAACAGCTCTTTGGACACTAGGAAAAAACCTTGTAGAGAGAGTAAAAAATT +TAACACCCATAGTAGGCCTAAAAGCAGCCACCAATTAAGAAAGCGTTCAAGCTCAACACC +CACTACCTAAAAAATCCCAAACATATAACTGAACTCCTCACACCCAATTGGACCAATCTA +TCACCCTATAGAAGAACTAATGTTAGTATAAGTAACATGAAAACATTCTCCTCCGCATAA +GCCTGCGTCAGATCAAAACACTGAACTGACAATTAACAGCCCAATATCTACAATCAACCA +ACAAGTCATTATTACCCTCACTGTCAACCCAACACAGGCATGCTCATAAGGAAAGGTTAA +AAAAAGTAAAAGGAACTCGGCAAACCTTACCCCGCCTGTTTACCAAAAACATCACCTCTA +GCATCACCAGTATTAGAGGCACCGCCTGCCCAGTGACACATGTTTAACGGCCGCGGTACC +CTAACCGTGCAaaggtagcataatcacttgttccttaaatagggacctgtatgaatggct +ccacgagggttcagctgtctcttacttttaaccagtgaaattgacctgcccgtgaagagg +cgggcatgacacagcaagacgagaagaccctatggagctttaatttaTTAATGCAAACAG +TACCTAACAAACCCACAGGTCCTAAACTACCAAACCTGCATTAAAAATTTCGGTTGGGGC +GACCTCGGAGCAGAACCCAACCTCCGAGCAGTACATGCTAAGACTTCACCAGTCAAAGCG +AACTACTATACTCAATTGATCCAATAACTTGACCAACGGAACAAGTTACCCTAGGGATAA +CAGCGCAATCCTATTCTAGAGTCCATATCAACAATAGGGTTTACGACCTCGATGTTGGAT +CAGGACATCCCGATGGTGCAGCCGCTATTAAAGGTTCGTTTGTTCAACGATTAAAGTCCT +ACGTGATCTGAGTTCAGACCGGAGTAATCCAGGTCGGTTTCTATCTACTTCAAATTCCTC +CCTGTACGAAAGGACAAGAGAAATAAGGCCTACTTCACAAAGCGCCTTCCCCCGTAAATG +ATATCATCTCAACTTAGTATTATACCCACACCCACCCAAGAACAGGGTTTgttaagatgg +cagagcccggtaatcgcataaaacttaaaactttacagtcagaggttcaattcctcttct +taacaacaTACCCATGGCCAACCTCCTACTCCTCATTGTACCCATTCTAATCGCAATGGC +ATTCCTAATGCTTACCGAACGAAAAATTCTAGGCTATATACAACTACGCAAAGGCCCCAA +CGTTGTAGGCCCCTACGGGCTACTACAACCCTTCGCTGACGCCATAAAACTCTTCACCAA +AGAGCCCCTAAAACCCGCCACATCTACCATCACCCTCTACATCACCGCCCCGACCTTAGC +TCTCACCATCGCTCTTCTACTATGAACCCCCCTCCCCATACCCAACCCCCTGGTCAACCT +CAACCTAGGCCTCCTATTTATTCTAGCCACCTCTAGCCTAGCCGTTTACTCAATCCTCTG +ATCAGGGTGAGCATCAAACTCAAACTACGCCCTGATCGGCGCACTGCGAGCAGTAGCCCA +AACAATCTCATATGAAGTCACCCTAGCCATCATTCTACTATCAACATTACTAATAAGTGG +CTCCTTTAACCTCTCCACCCTTATCACAACACAAGAACACCTCTGATTACTCCTGCCATC +ATGACCCTTGGCCATAATATGATTTATCTCCACACTAGCAGAGACCAACCGAACCCCCTT +CGACCTTGCCGAAGGGGAGTCCGAACTAGTCTCAGGCTTCAACATCGAATACGCCGCAGG +CCCCTTCGCCCTATTCTTCATAGCCGAATACACAAACATTATTATAATAAACACCCTCAC +CACTACAATCTTCCTAGGAACAACATATGACGCACTCTCCCCTGAACTCTACACAACATA +TTTTGTCACCAAGACCCTACTTCTAACCTCCCTGTTCTTATGAATTCGAACAGCATACCC +CCGATTCCGCTACGACCAACTCATACACCTCCTATGAAAAAACTTCCTACCACTCACCCT +AGCATTACTTATATGATATGTCTCCATACCCATTACAATCTCCAGCATTCCCCCTCAAAC +CTAAGAAATATGTCTGATAAAAGAGTTACTTTGATAGAGTAAATAATAGGAGCTTAAACC +CCCTTATTTctaggactatgagaatcgaacccatccctgagaatccaaaattctccgtgc +cacctatcacaccccatcctaAAGTAAGGTCAGCTAAATAAGCTATCGGGCCCATACCCC +GAAAATGTTGGTTATACCCTTCCCGTACTAATTAATCCCCTGGCCCAACCCGTCATCTAC +TCTACCATCTTTGCAGGCACACTCATCACAGCGCTAAGCTCGCACTGATTTTTTACCTGA +GTAGGCCTAGAAATAAACATGCTAGCTTTTATTCCAGTTCTAACCAAAAAAATAAACCCT +CGTTCCACAGAAGCTGCCATCAAGTATTTCCTCACGCAAGCAACCGCATCCATAATCCTT +CTAATAGCTATCCTCTTCAACAATATACTCTCCGGACAATGAACCATAACCAATACTACC +AATCAATACTCATCATTAATAATCATAATGGCTATAGCAATAAAACTAGGAATAGCCCCC +TTTCACTTCTGAGTCCCAGAGGTTACCCAAGGCACCCCTCTGACATCCGGCCTGCTTCTT +CTCACATGACAAAAACTAGCCCCCATCTCAATCATATACCAAATCTCTCCCTCACTAAAC +GTAAGCCTTCTCCTCACTCTCTCAATCTTATCCATCATAGCAGGCAGTTGAGGTGGATTA +AACCAAACCCAGCTACGCAAAATCTTAGCATACTCCTCAATTACCCACATAGGATGAATA +ATAGCAGTTCTACCGTACAACCCTAACATAACCATTCTTAATTTAACTATTTATATTATC +CTAACTACTACCGCATTCCTACTACTCAACTTAAACTCCAGCACCACGACCCTACTACTA +TCTCGCACCTGAAACAAGCTAACATGACTAACACCCTTAATTCCATCCACCCTCCTCTCC +CTAGGAGGCCTGCCCCCGCTAACCGGCTTTTTGCCCAAATGGGCCATTATCGAAGAATTC +ACAAAAAACAATAGCCTCATCATCCCCACCATCATAGCCACCATCACCCTCCTTAACCTC +TACTTCTACCTACGCCTAATCTACTCCACCTCAATCACACTACTCCCCATATCTAACAAC +GTAAAAATAAAATGACAGTTTGAACATACAAAACCCACCCCATTCCTCCCCACACTCATC +GCCCTTACCACGCTACTCCTACCTATCTCCCCTTTTATACTAATAATCTTATAGAAATTT +AGGTTAAATACAGACCAAGAGCCTTCAAAGCCCTCAGTAAGTTGCAATACTTAATTTCTG +CAACAGCTAAGGACTGCAAAACCCCACTCTGCATCAACTGAACGCAAATCAGCCACTTTA +ATTAAGCTAAGCCCTTACTAGACCAATGGGACTTAAACCCACAAACACTTAGTTAACAGC +TAAGCACCCTAATCAACTGGCTTCAATCTACTTCTCCCGCCGCCGGGAAAAAAGGCGGGA +GAAGCCCCGGCAGGTTTGAAGCTGCTTCTTCGAATTTGCAATTCAATATGAAAATCACCT +CGGAGCTGGTAAAAAGAGGCCTAACCCCTGTCTTTAGATTTACAGTCCAATGCTTCACTC +AGCCATTTTACCTCACCCCCACTGATGTTCGCCGACCGTTGACTATTCTCTACAAACCAC +AAAGACATTGGAACACTATACCTATTATTCGGCGCATGAGCTGGAGTCCTAGGCACAGCT +CTAAGCCTCCTTATTCGAGCCGAGCTGGGCCAGCCAGGCAACCTTCTAGGTAACGACCAC +ATCTACAACGTTATCGTCACAGCCCATGCATTTGTAATAATCTTCTTCATAGTAATACCC +ATCATAATCGGAGGCTTTGGCAACTGACTAGTTCCCCTAATAATCGGTGCCCCCGATATG +GCGTTTCCCCGCATAAACAACATAAGCTTCTGACTCTTACCTCCCTCTCTCCTACTCCTG +CTCGCATCTGCTATAGTGGAGGCCGGAGCAGGAACAGGTTGAACAGTCTACCCTCCCTTA +GCAGGGAACTACTCCCACCCTGGAGCCTCCGTAGACCTAACCATCTTCTCCTTACACCTA +GCAGGTGTCTCCTCTATCTTAGGGGCCATCAATTTCATCACAACAATTATCAATATAAAA +CCCCCTGCCATAACCCAATACCAAACGCCCCTCTTCGTCTGATCCGTCCTAATCACAGCA +GTCCTACTTCTCCTATCTCTCCCAGTCCTAGCTGCTGGCATCACTATACTACTAACAGAC +CGCAACCTCAACACCACCTTCTTCGACCCCGCCGGAGGAGGAGACCCCATTCTATACCAA +CACCTATTCTGATTTTTCGGTCACCCTGAAGTTTATATTCTTATCCTACCAGGCTTCGGA +ATAATCTCCCATATTGTAACTTACTACTCCGGAAAAAAAGAACCATTTGGATACATAGGT +ATGGTCTGAGCTATGATATCAATTGGCTTCCTAGGGTTTATCGTGTGAGCACACCATATA +TTTACAGTAGGAATAGACGTAGACACACGAGCATATTTCACCTCCGCTACCATAATCATC +GCTATCCCCACCGGCGTCAAAGTATTTAGCTGACTCGCCACACTCCACGGAAGCAATATG +AAATGATCTGCTGCAGTGCTCTGAGCCCTAGGATTCATCTTTCTTTTCACCGTAGGTGGC +CTGACTGGCATTGTATTAGCAAACTCATCACTAGACATCGTACTACACGACACGTACTAC +GTTGTAGCTCACTTCCACTATGTCCTATCAATAGGAGCTGTATTTGCCATCATAGGAGGC +TTCATTCACTGATTTCCCCTATTCTCAGGCTACACCCTAGACCAAACCTACGCCAAAATC +CATTTCACTATCATATTCATCGGCGTAAATCTAACTTTCTTCCCACAACACTTTCTCGGC +CTATCCGGAATGCCCCGACGTTACTCGGACTACCCCGATGCATACACCACATGAAACATC +CTATCATCTGTAGGCTCATTCATTTCTCTAACAGCAGTAATATTAATAATTTTCATGATT +TGAGAAGCCTTCGCTTCGAAGCGAAAAGTCCTAATAGTAGAAGAACCCTCCATAAACCTG +GAGTGACTATATGGATGCCCCCCACCCTACCACACATTCGAAGAACCCGTATACATAAAA +TCTAGACAaaaaaggaaggaatcgaaccccccaaagctggtttcaagccaaccccatggc +ctccatgactttttcAAAAAGGTATTAGAAAAACCATTTCATAACTTTGTCAAAGTTAAA +TTATAGGCTAAATCCTATATATCTTAATGGCACATGCAGCGCAAGTAGGTCTACAAGACG +CTACTTCCCCTATCATAGAAGAGCTTATCACCTTTCATGATCACGCCCTCATAATCATTT +TCCTTATCTGCTTCCTAGTCCTGTATGCCCTTTTCCTAACACTCACAACAAAACTAACTA +ATACTAACATCTCAGACGCTCAGGAAATAGAAACCGTCTGAACTATCCTGCCCGCCATCA +TCCTAGTCCTCATCGCCCTCCCATCCCTACGCATCCTTTACATAACAGACGAGGTCAACG +ATCCCTCCCTTACCATCAAATCAATTGGCCACCAATGGTACTGAACCTACGAGTACACCG +ACTACGGCGGACTAATCTTCAACTCCTACATACTTCCCCCATTATTCCTAGAACCAGGCG +ACCTGCGACTCCTTGACGTTGACAATCGAGTAGTACTCCCGATTGAAGCCCCCATTCGTA +TAATAATTACATCACAAGACGTCTTGCACTCATGAGCTGTCCCCACATTAGGCTTAAAAA +CAGATGCAATTCCCGGACGTCTAAACCAAACCACTTTCACCGCTACACGACCGGGGGTAT +ACTACGGTCAATGCTCTGAAATCTGTGGAGCAAACCACAGTTTCATGCCCATCGTCCTAG +AATTAATTCCCCTAAAAATCTTTGAAATAGGGCCCGTATTTACCCTATAGCACCCCCTCT +ACCCCCTCTAGAGCCCACTGTAAAGCTAACTTAGCATTAACCTTTTAAGTTAAAGATTAA +GAGAACCAACACCTCTTTACAGTGAAATGCCCCAACTAAATACTACCGTATGGCCCACCA +TAATTACCCCCATACTCCTTACACTATTCCTCATCACCCAACTAAAAATATTAAACACAA +ACTACCACCTACCTCCCTCACCAAAGCCCATAAAAATAAAAAATTATAACAAACCCTGAG +AACCAAAATGAACGAAAATCTGTTCGCTTCATTCATTGCCCCCACAATCCTAGGCCTACC +CGCCGCAGTACTGATCATTCTATTTCCCCCTCTATTGATCCCCACCTCCAAATATCTCAT +CAACAACCGACTAATCACCACCCAACAATGACTAATCAAACTAACCTCAAAACAAATGAT +AGCCATACACAACACTAAAGGACGAACCTGATCTCTTATACTAGTATCCTTAATCATTTT +TATTGCCACAACTAACCTCCTCGGACTCCTGCCTCACTCATTTACACCAACCACCCAACT +ATCTATAAACCTAGCCATGGCCATCCCCTTATGAGCGGGCGCAGTGATTATAGGCTTTCG +CTCTAAGATTAAAAATGCCCTAGCCCACTTCTTACCACAAGGCACACCTACACCCCTTAT +CCCCATACTAGTTATTATCGAAACCATCAGCCTACTCATTCAACCAATAGCCCTGGCCGT +ACGCCTAACCGCTAACATTACTGCAGGCCACCTACTCATGCACCTAATTGGAAGCGCCAC +CCTAGCAATATCAACCATTAACCTTCCCTCTACACTTATCATCTTCACAATTCTAATTCT +ACTGACTATCCTAGAAATCGCTGTCGCCTTAATCCAAGCCTACGTTTTCACACTTCTAGT +AAGCCTCTACCTGCACGACAACACATAATGACCCACCAATCACATGCCTATCATATAGTA +AAACCCAGCCCATGACCCCTAACAGGGGCCCTCTCAGCCCTCCTAATGACCTCCGGCCTA +GCCATGTGATTTCACTTCCACTCCATAACGCTCCTCATACTAGGCCTACTAACCAACACA +CTAACCATATACCAATGGTGGCGCGATGTAACACGAGAAAGCACATACCAAGGCCACCAC +ACACCACCTGTCCAAAAAGGCCTTCGATACGGGATAATCCTATTTATTACCTCAGAAGTT +TTTTTCTTCGCAGGATTTTTCTGAGCCTTTTACCACTCCAGCCTAGCCCCTACCCCCCAA +CTAGGAGGGCACTGGCCCCCAACAGGCATCACCCCGCTAAATCCCCTAGAAGTCCCACTC +CTAAACACATCCGTATTACTCGCATCAGGAGTATCAATCACCTGAGCTCACCATAGTCTA +ATAGAAAACAACCGAAACCAAATAATTCAAGCACTGCTTATTACAATTTTACTGGGTCTC +TATTTTACCCTCCTACAAGCCTCAGAGTACTTCGAGTCTCCCTTCACCATTTCCGACGGC +ATCTACGGCTCAACATTTTTTGTAGCCACAGGCTTCCACGGACTTCACGTCATTATTGGC +TCAACTTTCCTCACTATCTGCTTCATCCGCCAACTAATATTTCACTTTACATCCAAACAT +CACTTTGGCTTCGAAGCCGCCGCCTGATACTGGCATTTTGTAGATGTGGTTTGACTATTT +CTGTATGTCTCCATCTATTGATGAGGGTCTTACTCTTTTAGTATAAATAGTACCGTTAAC +TTCCAATTAACTAGTTTTGACAACATTCAAAAAAGAGTAATAAACTTCGCCTTAATTTTA +ATAATCAACACCCTCCTAGCCTTACTACTAATAATTATTACATTTTGACTACCACAACTC +AACGGCTACATAGAAAAATCCACCCCTTACGAGTGCGGCTTCGACCCTATATCCCCCGCC +CGCGTCCCTTTCTCCATAAAATTCTTCTTAGTAGCTATTACCTTCTTATTATTTGATCTA +GAAATTGCCCTCCTTTTACCCCTACCATGAGCCCTACAAACAACTAACCTGCCACTAATA +GTTATGTCATCCCTCTTATTAATCATCATCCTAGCCCTAAGTCTGGCCTATGAGTGACTA +CAAAAAGGATTAGACTGAGCCGAATTGGTATATAGTTTAAACAAAACGAATGATTTCGAC +TCATTAAATTATGATAATCATATTTACCAAATGCCCCTCATTTACATAAATATTATACTA +GCATTTACCATCTCACTTCTAGGAATACTAGTATATCGCTCACACCTCATATCCTCCCTA +CTATGCCTAGAAGGAATAATACTATCGCTGTTCATTATAGCTACTCTCATAACCCTCAAC +ACCCACTCCCTCTTAGCCAATATTGTGCCTATTGCCATACTAGTCTTTGCCGCCTGCGAA +GCAGCGGTGGGCCTAGCCCTACTAGTCTCAATCTCCAACACATATGGCCTAGACTACGTA +CATAACCTAAACCTACTCCAATGCTAAAACTAATCGTCCCAACAATTATATTACTACCAC +TGACATGACTTTCCAAAAAGCACATAATTTGAATCAACACAACCACCCACAGCCTAATTA +TTAGCATCATCCCCCTACTATTTTTTAACCAAATCAACAACAACCTATTTAGCTGTTCCC +CAACCTTTTCCTCCGACCCCCTAACAACCCCCCTCCTAATACTAACTACCTGACTCCTAC +CCCTCACAATCATGGCAAGCCAACGCCACTTATCCAGCGAACCACTATCACGAAAAAAAC +TCTACCTCTCTATACTAATCTCCCTACAAATCTCCTTAATTATAACATTCACAGCCACAG +AACTAATCATATTTTATATCTTCTTCGAAACCACACTTATCCCCACCTTGGCTATCATCA +CCCGATGAGGCAACCAGCCAGAACGCCTGAACGCAGGCACATACTTCCTATTCTACACCC +TAGTAGGCTCCCTTCCCCTACTCATCGCACTAATTTACACTCACAACACCCTAGGCTCAC +TAAACATTCTACTACTCACTCTCACTGCCCAAGAACTATCAAACTCCTGAGCCAACAACT +TAATATGACTAGCTTACACAATAGCTTTTATAGTAAAGATACCTCTTTACGGACTCCACT +TATGACTCCCTAAAGCCCATGTCGAAGCCCCCATCGCTGGGTCAATAGTACTTGCCGCAG +TACTCTTAAAACTAGGCGGCTATGGTATAATACGCCTCACACTCATTCTCAACCCCCTGA +CAAAACACATAGCCTACCCCTTCCTTGTACTATCCCTATGAGGCATAATTATAACAAGCT +CCATCTGCCTACGACAAACAGACCTAAAATCGCTCATTGCATACTCTTCAATCAGCCACA +TAGCCCTCGTAGTAACAGCCATTCTCATCCAAACCCCCTGAAGCTTCACCGGCGCAGTCA +TTCTCATAATCGCCCACGGACTCACATCCTCATTACTATTCTGCCTAGCAAACTCAAACT +ACGAACGCACTCACAGTCGCATCATAATCCTCTCTCAAGGACTTCAAACTCTACTCCCAC +TAATAGCTTTTTGATGACTTCTAGCAAGCCTCGCTAACCTCGCCTTACCCCCCACTATTA +ACCTACTGGGAGAACTCTCTGTGCTAGTAACCACGTTCTCCTGATCAAATATCACTCTCC +TACTTACAGGACTCAACATACTAGTCACAGCCCTATACTCCCTCTACATATTTACCACAA +CACAATGGGGCTCACTCACCCACCACATTAACAACATAAAACCCTCATTCACACGAGAAA +ACACCCTCATGTTCATACACCTATCCCCCATTCTCCTCCTATCCCTCAACCCCGACATCA +TTACCGGGTTTTCCTCTTGTAAATATAGTTTAACCAAAACATCAGATTGTGAATCTGACA +ACAGAGGCTTACGACCCCTTATTTACCGAGAAAGCTCACAAGAACTGCTAACTCATGCCC +CCATGTCTAACAACATGGCTTTCTCAACTTTTAAAGGATAACAGCTATCCATTGGTCTTA +GGCCCCAAAAATTTTGGTGCAACTCCAAATAAAAGTAATAACCATGCACACTACTATAAC +CACCCTAACCCTGACTTCCCTAATTCCCCCCATCCTTACCACCCTCGTTAACCCTAACAA +AAAAAACTCATACCCCCATTATGTAAAATCCATTGTCGCATCCACCTTTATTATCAGTCT +CTTCCCCACAACAATATTCATGTGCCTAGACCAAGAAGTTATTATCTCGAACTGACACTG +AGCCACAACCCAAACAACCCAGCTCTCCCTAAGCTTCAAACTAGACTACTTCTCCATAAT +ATTCATCCCTGTAGCATTGTTCGTTACATGGTCCATCATAGAATTCTCACTGTGATATAT +AAACTCAGACCCAAACATTAATCAGTTCTTCAAATATCTACTCATTTTCCTAATTACCAT +ACTAATCTTAGTTACCGCTAACAACCTATTCCAACTGTTCATCGGCTGAGAGGGCGTAGG +AATTATATCCTTCTTGCTCATCAGTTGATGATACGCCCGAGCAGATGCCAACACAGCAGC +CATTCAAGCAGTCCTATACAACCGTATCGGCGATATCGGTTTCATCCTCGCCTTAGCATG +ATTTATCCTACACTCCAACTCATGAGACCCACAACAAATAGCCCTTCTAAACGCTAATCC +AAGCCTCACCCCACTACTAGGCCTCCTCCTAGCAGCAGCAGGCAAATCAGCCCAATTAGG +TCTCCACCCCTGACTCCCCTCAGCCATAGAAGGCCCCACCCCAGTCTCAGCCCTACTCCA +CTCAAGCACTATAGTTGTAGCAGGAATCTTCTTACTCATCCGCTTCCACCCCCTAGCAGA +AAATAGCCCACTAATCCAAACTCTAACACTATGCTTAGGCGCTATCACCACTCTGTTCGC +AGCAGTCTGCGCCCTTACACAAAATGACATCAAAAAAATCGTAGCCTTCTCCACTTCAAG +TCAACTAGGACTCATAATAGTTACAATCGGCATCAACCAACCACACCTAGCATTCCTGCA +CATCTGTACCCACGCCTTCTTCAAAGCCATACTATTTATGTGCTCCGGGTCCATCATCCA +CAACCTTAACAATGAACAAGATATTCGAAAAATAGGAGGACTACTCAAAACCATACCTCT +CACTTCAACCTCCCTCACCATTGGCAGCCTAGCATTAGCAGGAATACCTTTCCTCACAGG +TTTCTACTCCAAAGACCACATCATCGAAACCGCAAACATATCATACACAAACGCCTGAGC +CCTATCTATTACTCTCATCGCTACCTCCCTGACAAGCGCCTATAGCACTCGAATAATTCT +TCTCACCCTAACAGGTCAACCTCGCTTCCCCACCCTTACTAACATTAACGAAAATAACCC +CACCCTACTAAACCCCATTAAACGCCTGGCAGCCGGAAGCCTATTCGCAGGATTTCTCAT +TACTAACAACATTTCCCCCGCATCCCCCTTCCAAACAACAATCCCCCTCTACCTAAAACT +CACAGCCCTCGCTGTCACTTTCCTAGGACTTCTAACAGCCCTAGACCTCAACTACCTAAC +CAACAAACTTAAAATAAAATCCCCACTATGCACATTTTATTTCTCCAACATACTCGGATT +CTACCCTAGCATCACACACCGCACAATCCCCTATCTAGGCCTTCTTACGAGCCAAAACCT +GCCCCTACTCCTCCTAGACCTAACCTGACTAGAAAAGCTATTACCTAAAACAATTTCACA +GCACCAAATCTCCACCTCCATCATCACCTCAACCCAAAAAGGCATAATTAAACTTTACTT +CCTCTCTTTCTTCTTCCCACTCATCCTAACCCTACTCCTAATCACATAACCTATTCCCCC +GAGCAATCTCAATTACAATATATACACCAACAAACAATGTTCAACCAGTAACCACTACTA +ATCAACGCCCATAATCATACAAAGCCCCCGCACCAATAGGATCCTCCCGAATCAACCCTG +ACCCCTCTCCTTCATAAATTATTCAGCTTCCTACACTATTAAAGTTTACCACAACCACCA +CCCCATCATACTCTTTCACCCACAGCACCAATCCTACCTCCATCGCTAACCCCACTAAAA +CACTCACCAAGACCTCAACCCCTGACCCCCATGCCTCAGGATACTCCTCAATAGCCATCG +CTGTAGTATATCCAAAGACAACCATCATTCCCCCTAAATAAATTAAAAAAACTATTAAAC +CCATATAACCTCCCCCAAAATTCAGAATAATAACACACCCGACCACACCGCTAACAATCA +GTACTAAACCCCCATAAATAGGAGAAGGCTTAGAAGAAAACCCCACAAACCCCATTACTA +AACCCACACTCAACAGAAACAAAGCATACATCATTATTCTCGCACGGACTACAACCACGA +CCAATGATATGAAAAACCATCGTTGTATTTCAACTACAAGAACACCAATGACCCCAATAC +GCAAAATTAACCCCCTAATAAAATTAATTAACCACTCATTCATCGACCTCCCCACCCCAT +CCAACATCTCCGCATGATGAAACTTCGGCTCACTCCTTGGCGCCTGCCTGATCCTCCAAA +TCACCACAGGACTATTCCTAGCCATACACTACTCACCAGACGCCTCAACCGCCTTTTCAT +CAATCGCCCACATCACTCGAGACGTAAATTATGGCTGAATCATCCGCTACCTTCACGCCA +ATGGCGCCTCAATATTCTTTATCTGCCTCTTCCTACACATCGGGCGAGGCCTATATTACG +GATCATTTCTCTACTCAGAAACCTGAAACATCGGCATTATCCTCCTGCTTGCAACTATAG +CAACAGCCTTCATAGGCTATGTCCTCCCGTGAGGCCAAATATCATTCTGAGGGGCCACAG +TAATTACAAACTTACTATCCGCCATCCCATACATTGGGACAGACCTAGTTCAATGAATCT +GAGGAGGCTACTCAGTAGACAGTCCCACCCTCACACGATTCTTTACCTTTCACTTCATCT +TACCCTTCATTATTGCAGCCCTAGCAGCACTCCACCTCCTATTCTTGCACGAAACGGGAT +CAAACAACCCCCTAGGAATCACCTCCCATTCCGATAAAATCACCTTCCACCCTTACTACA +CAATCAAAGACGCCCTCGGCTTACTTCTCTTCCTTCTCTCCTTAATGACATTAACACTAT +TCTCACCAGACCTCCTAGGCGACCCAGACAATTATACCCTAGCCAACCCCTTAAACACCC +CTCCCCACATCAAGCCCGAATGATATTTCCTATTCGCCTACACAATTCTCCGATCCGTCC +CTAACAAACTAGGAGGCGTCCTTGCCCTATTACTATCCATCCTCATCCTAGCAATAATCC +CCATCCTCCATATATCCAAACAACAAAGCATAATATTTCGCCCACTAAGCCAATCACTTT +ATTGACTCCTAGCCGCAGACCTCCTCATTCTAACCTGAATCGGAGGACAACCAGTAAGCT +ACCCTTTTACCATCATTGGACAAGTAGCATCCGTACTATACTTCACAACAATCCTAATCC +TAATACCAACTATCTCCCTAATTGAAAACAAAATACTCAAATGGGCCTGTCCTTGTAGTA +TAAACTAATACACCAGTCTTGTAAACCGGAGACGAAAACCTTTTTCCAAGGACAAATCAG +AGAAAAAGTCTTTAACTCCACCATTAGCACCCAAAGCTAAGATTCTAATTTAAACTATTC +TCTGTTCTTTCATGGGGAAGCAGATTTGGGTACCACCCAAGTATTGACTCACCCATCAAC +AACCGCTATGTATTTCGTACATTACTGCCAGCCACCATGAATATTGTACGGTACCATAAA +TACTTGACCACCTGTAGTACATAAAAACCCAACCCACATCAAACCCCCCCCCCCCATGCT +TACAAGCAAGTACAGCAATCAACCTTCAACTATCACACATCAACTGCAACTCCAAAGCCA +CCCCTCACCCACTAGGATACCAACAAACCTACCCACCCTTAACAGTACATAGTACATAAA +GTCATTTACCGTACATAGCACATTACAGTCAAATCCCTTCTCGTCCCCATGGATGACCCC +CCTCAGATAGGGGTCCCTTGACCACCATCCTCCGTGAAATCAATATCCCGCACAAGAGTG +CTACTCTCCTCGCTCCGGGCCCATAACACTTGGGGGTAGCTAAAGTGAACTGTATCCGAC +ATCTGGTTCCTACTTCAGGGCCATAAAGCCTAAATAGCCCACACGTTCCCCTTAAATAAG +ACATCACGATG \ No newline at end of file From 744d7c6cc4e1efcf6cac1fc7bd4e77cc7b5ef0c1 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Wed, 10 Oct 2018 17:59:38 -0400 Subject: [PATCH 02/20] forgot fai --- src/test/resources/htsjdk/samtools/one-contig.fasta.fai | 1 + 1 file changed, 1 insertion(+) create mode 100644 src/test/resources/htsjdk/samtools/one-contig.fasta.fai diff --git a/src/test/resources/htsjdk/samtools/one-contig.fasta.fai b/src/test/resources/htsjdk/samtools/one-contig.fasta.fai new file mode 100644 index 0000000000..d626db67d6 --- /dev/null +++ b/src/test/resources/htsjdk/samtools/one-contig.fasta.fai @@ -0,0 +1 @@ +chr1 16571 6 60 61 From 306c309c3def9a6c696c392620689cedc3647b9c Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Tue, 16 Oct 2018 14:59:12 -0400 Subject: [PATCH 03/20] add space --- src/main/java/htsjdk/samtools/TextTagCodec.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/htsjdk/samtools/TextTagCodec.java b/src/main/java/htsjdk/samtools/TextTagCodec.java index 6d38761106..c6bc98d2e8 100644 --- a/src/main/java/htsjdk/samtools/TextTagCodec.java +++ b/src/main/java/htsjdk/samtools/TextTagCodec.java @@ -99,7 +99,7 @@ private char getArrayType(final Object array, final boolean isUnsigned) { private String encodeArrayValue(final Object value) { final int length = Array.getLength(value); - if(length == 0){ + if (length == 0){ return ""; } final StringBuilder ret = new StringBuilder(Array.get(value, 0).toString()); From 8aabba8f430885e8965ef58eae49ee0d24efa261 Mon Sep 17 00:00:00 2001 From: Yossi Farjoun Date: Wed, 17 Oct 2018 14:17:52 -0400 Subject: [PATCH 04/20] space --- src/test/java/htsjdk/samtools/SAMRecordUnitTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java index 29de84229b..0db58be691 100644 --- a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java +++ b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java @@ -1188,7 +1188,7 @@ public void test_setAttribute_empty_array() { } @DataProvider - public Object[][] getEmptyArrays(){ + public Object[][] getEmptyArrays() { final String BAM = BamFileIoUtils.BAM_FILE_EXTENSION; final String SAM = IOUtil.SAM_FILE_EXTENSION; final String CRAM = CramIO.CRAM_FILE_EXTENSION; From 14ff9e4afe4b6a59426014700c4748da564afa17 Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Wed, 17 Oct 2018 14:18:06 -0400 Subject: [PATCH 05/20] space --- src/test/java/htsjdk/samtools/SAMRecordUnitTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java index 0db58be691..96767cccfd 100644 --- a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java +++ b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java @@ -1218,7 +1218,7 @@ public void testWriteSamWithEmptyArray(Object emptyArray, Class arrayClass, S final String arrayTag = "XA"; final SAMRecordSetBuilder samRecords = new SAMRecordSetBuilder(); - samRecords.addFrag("Read",0,100, false); + samRecords.addFrag("Read", 0, 100, false); final SAMRecord record = samRecords.getRecords().iterator().next(); record.setAttribute(arrayTag, emptyArray); checkArrayIsEmpty(arrayTag, record, arrayClass); From ef8c7a2de81e3a986f5311c5aa9d5e2acf08fb47 Mon Sep 17 00:00:00 2001 From: Yossi Farjoun Date: Wed, 17 Oct 2018 14:18:45 -0400 Subject: [PATCH 06/20] lowercase tag --- src/test/java/htsjdk/samtools/SAMRecordUnitTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java index 96767cccfd..9dddbae5db 100644 --- a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java +++ b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java @@ -1216,7 +1216,7 @@ public void testWriteSamWithEmptyArray(Object emptyArray, Class arrayClass, S Assert.assertEquals(emptyArray.getClass(), arrayClass); Assert.assertEquals(Array.getLength(emptyArray), 0); - final String arrayTag = "XA"; + final String arrayTag = "xa"; final SAMRecordSetBuilder samRecords = new SAMRecordSetBuilder(); samRecords.addFrag("Read", 0, 100, false); final SAMRecord record = samRecords.getRecords().iterator().next(); From 5696fb499bf3d0a036b7085bbf83581d62b02380 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Wed, 17 Oct 2018 14:21:01 -0400 Subject: [PATCH 07/20] responding to comments * adding explicit test dependency on a recent version of guava, we already have a transitive test dependency on an older version so this shouldn't be a big deal (unless of course it breaks something unexpected) * adding new method for combining dataproviders to TestNGUtils --- build.gradle | 1 + .../java/htsjdk/samtools/TextTagCodec.java | 23 ++++- .../htsjdk/samtools/SAMRecordUnitTest.java | 37 ++++---- src/test/java/htsjdk/tribble/TestUtils.java | 9 ++ src/test/java/htsjdk/utils/TestNGUtils.java | 39 ++++++++ .../htsjdk/utils/TestNGUtilsUnitTest.java | 94 +++++++++++++++++++ 6 files changed, 178 insertions(+), 25 deletions(-) create mode 100644 src/test/java/htsjdk/utils/TestNGUtilsUnitTest.java diff --git a/build.gradle b/build.gradle index 7753a6f386..25a8e7d07c 100644 --- a/build.gradle +++ b/build.gradle @@ -43,6 +43,7 @@ dependencies { testRuntime 'org.pegdown:pegdown:1.6.0' // Necessary for generating HTML reports with ScalaTest testCompile "org.testng:testng:6.14.3" testCompile "com.google.jimfs:jimfs:1.1" + testCompile "com.google.guava:guava:26.0-jre" } sourceCompatibility = 1.8 diff --git a/src/main/java/htsjdk/samtools/TextTagCodec.java b/src/main/java/htsjdk/samtools/TextTagCodec.java index c6bc98d2e8..e416aa6dc0 100644 --- a/src/main/java/htsjdk/samtools/TextTagCodec.java +++ b/src/main/java/htsjdk/samtools/TextTagCodec.java @@ -44,6 +44,11 @@ public class TextTagCodec { // 3 fields for non-empty strings 2 fields if the string is empty. private static final int NUM_TAG_FIELDS = 3; + private static final byte[] EMPTY_BYTE_ARRAY = new byte[0]; + private static final short[] EMPTY_SHORT_ARRAY = new short[0]; + private static final int[] EMPTY_INT_ARRAY = new int[0]; + private static final float[] EMPTY_FLOAT_ARRAY = new float[0]; + /** * This is really a local variable of decode(), but allocated here to reduce allocations. */ @@ -327,11 +332,19 @@ private Object covertStringArrayToObject(final String stringVal) { } private static Object createEmptyArray(char elementType) { - switch( Character.toLowerCase(elementType) ) { - case 'c': { return new byte[0]; } - case 's': { return new short[0]; } - case 'i': { return new int[0]; } - case 'f': { return new float[0]; } + switch ( elementType ) { + case 'c': + case 'C': + return EMPTY_BYTE_ARRAY; + case 's': + case 'S': + return EMPTY_SHORT_ARRAY; + case 'i': + case 'I': + return EMPTY_INT_ARRAY; + case 'f': + //note that F is not a valid option since there is no signed/unsigned float distinction + return EMPTY_FLOAT_ARRAY; default: { throw new SAMFormatException("Unrecognized array tag element type: " + elementType); } } } diff --git a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java index 9dddbae5db..03d0a1bd19 100644 --- a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java +++ b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java @@ -29,6 +29,7 @@ import htsjdk.samtools.util.BinaryCodec; import htsjdk.samtools.util.IOUtil; import htsjdk.samtools.util.TestUtil; +import htsjdk.utils.TestNGUtils; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -1187,31 +1188,27 @@ public void test_setAttribute_empty_array() { } - @DataProvider - public Object[][] getEmptyArrays() { - final String BAM = BamFileIoUtils.BAM_FILE_EXTENSION; - final String SAM = IOUtil.SAM_FILE_EXTENSION; - final String CRAM = CramIO.CRAM_FILE_EXTENSION; + private static Object[][] getEmptyArrays() { return new Object[][]{ - {new int[0], int[].class, BAM}, - {new short[0], short[].class, BAM}, - {new byte[0], byte[].class, BAM}, - {new float[0], float[].class, BAM}, - - {new int[0], int[].class, SAM}, - {new short[0], short[].class, SAM}, - {new byte[0], byte[].class, SAM}, - {new float[0], float[].class, SAM}, - - {new int[0], int[].class, CRAM}, - {new short[0], short[].class, CRAM}, - {new byte[0], byte[].class, CRAM}, - {new float[0], float[].class, CRAM}, + {new int[0], int[].class}, + {new short[0], short[].class}, + {new byte[0], byte[].class}, + {new float[0], float[].class}, + }; + } + private static Object[][] getFileExtension(){ + return new Object[][]{ + {BamFileIoUtils.BAM_FILE_EXTENSION}, {IOUtil.SAM_FILE_EXTENSION}, {CramIO.CRAM_FILE_EXTENSION} }; } - @Test(dataProvider = "getEmptyArrays") + @DataProvider + public Object[][] getEmptyArraysAndExtensions(){ + return TestNGUtils.cartesianProduct(getEmptyArrays(), getFileExtension()); + } + + @Test(dataProvider = "getEmptyArraysAndExtensions") public void testWriteSamWithEmptyArray(Object emptyArray, Class arrayClass, String fileExtension) throws IOException { Assert.assertEquals(emptyArray.getClass(), arrayClass); Assert.assertEquals(Array.getLength(emptyArray), 0); diff --git a/src/test/java/htsjdk/tribble/TestUtils.java b/src/test/java/htsjdk/tribble/TestUtils.java index b2ca9e7a50..d958e14306 100644 --- a/src/test/java/htsjdk/tribble/TestUtils.java +++ b/src/test/java/htsjdk/tribble/TestUtils.java @@ -18,12 +18,19 @@ package htsjdk.tribble; +import htsjdk.utils.TestNGUtils; +import org.testng.Assert; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + import java.io.IOException; import java.net.URISyntaxException; import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; +import java.util.List; /** * User: jacob @@ -57,4 +64,6 @@ public static Path getTribbleFileInJimfs(String vcf, String index, FileSystem fi } return Files.copy(vcfPath, vcfDestination); } + + } diff --git a/src/test/java/htsjdk/utils/TestNGUtils.java b/src/test/java/htsjdk/utils/TestNGUtils.java index cc7dd6c558..4bcdecf3c8 100644 --- a/src/test/java/htsjdk/utils/TestNGUtils.java +++ b/src/test/java/htsjdk/utils/TestNGUtils.java @@ -1,11 +1,13 @@ package htsjdk.utils; +import com.google.common.collect.Lists; import org.testng.annotations.DataProvider; import org.testng.collections.Sets; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.*; +import java.util.stream.Collectors; /** * Small class implementing some utility functions that are useful for test and interfacing with the TestNG framework. @@ -40,4 +42,41 @@ public static Iterator getDataProviders(final String packageName) { return data.iterator(); } + + /** + * @return the cartesian product of two or more DataProviders than can be used as a new dataprovider + */ + public static Object[][] cartesianProduct(Object[][] ... dataProviders){ + final List[] lists = Arrays.stream(dataProviders) + .map(TestNGUtils::nestedArraysToNestedLists) + .toArray(List[]::new); + + @SuppressWarnings("unchecked") + final List>> product = Lists.cartesianProduct(lists); + final List> mergeProduct = product.stream() + .map((List> list) -> { + List result = new ArrayList<>(); + list.forEach(result::addAll); + return result; + }).collect(Collectors.toList()); + return nestedListsToNestedArrays(mergeProduct); + } + + /** + * @param dataProvider a nested Object array + * @return an equivalent nested List + */ + public static List> nestedArraysToNestedLists(Object[][] dataProvider){ + return Arrays.stream(dataProvider) + .map(Arrays::asList) + .collect(Collectors.toList()); + } + + /** + * @param lists a nested List + * @return an equivalent nested array + */ + public static Object[][] nestedListsToNestedArrays(List> lists){ + return lists.stream().map(List::toArray).toArray(Object[][]::new); + } } diff --git a/src/test/java/htsjdk/utils/TestNGUtilsUnitTest.java b/src/test/java/htsjdk/utils/TestNGUtilsUnitTest.java new file mode 100644 index 0000000000..e1718e014f --- /dev/null +++ b/src/test/java/htsjdk/utils/TestNGUtilsUnitTest.java @@ -0,0 +1,94 @@ +package htsjdk.utils; + +import org.testng.Assert; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import java.util.Arrays; +import java.util.List; + +public class TestNGUtilsUnitTest { + + @DataProvider + public Object[][] getArraysAndLists() { + return new Object[][]{ + { + new Object[][]{ + {1, 2}, + {3, 4} + } + , Arrays.asList( + Arrays.asList(1, 2), + Arrays.asList(3, 4) + ) + }, + { + new Object[][]{ + {1} + }, + Arrays.asList( + Arrays.asList(1) + ) + }, + { + new Object[][]{ + {1,2,3}, + {4,5,6} + }, + Arrays.asList( + Arrays.asList(1,2,3), + Arrays.asList(4,5,6) + ) + }, + { + new Object[][]{ + {1}, + {2}, + {3}, + {4} + }, + Arrays.asList( + Arrays.asList(1), + Arrays.asList(2), + Arrays.asList(3), + Arrays.asList(4) + ) + } + }; + } + + @Test(dataProvider = "getArraysAndLists") + public void testObjectsToLists(Object[][] objects, List> lists){ + Assert.assertEquals(TestNGUtils.nestedArraysToNestedLists(objects), lists); + } + + @Test(dataProvider = "getArraysAndLists") + public void testListsToArrays(Object[][] objects, List> lists){ + final Object[][] convertedObjects = TestNGUtils.nestedListsToNestedArrays(lists); + assertNestedArraysEqual(objects, convertedObjects); + } + + private static void assertNestedArraysEqual(Object[][] objects, Object[][] convertedObjects) { + for(int i = 0; i < objects.length; i++){ + Assert.assertEquals(convertedObjects[i], objects[i]); + } + } + + @DataProvider + public Object[][] getDataProviders(){ + return new Object[][]{ + { new Object[][]{ {1,2} }, new Object[][]{{3},{ 4}}, new Object[][]{{1, 2, 3}, {1, 2, 4}}}, + { new Object[][]{ {1} }, new Object[][]{{2}}, new Object[][]{{1,2}}}, + { new Object[][]{ {1},{2},{3} }, new Object[][]{ {'a', 'b'}, {'c','d'} }, + new Object[][]{{1, 'a', 'b'}, {1, 'c', 'd'}, + {2, 'a', 'b'}, {2, 'c', 'd'}, + {3, 'a', 'b'}, {3, 'c', 'd'}}} + }; + } + + @Test(dataProvider = "getDataProviders") + public void testProduct(Object[][] dataProvider1, Object[][] dataProvider2, Object[][] expectedResult){ + final Object[][] actual = TestNGUtils.cartesianProduct(dataProvider1, dataProvider2); + assertNestedArraysEqual( actual, expectedResult); + } +} From 133370ad00cb8488b86e9cc376dc77cb17d3d7a6 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Wed, 17 Oct 2018 14:25:28 -0400 Subject: [PATCH 08/20] lowercase --- src/test/java/htsjdk/samtools/SAMRecordUnitTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java index 03d0a1bd19..5bb236474b 100644 --- a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java +++ b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java @@ -1175,7 +1175,7 @@ public void testHasAttribute(final SAMRecord samRecord, final String tag, final @Test public void test_setAttribute_empty_array() { final SAMFileHeader header = new SAMFileHeader(); - final String arrayTag = "XA"; + final String arrayTag = "xa"; final SAMRecord record = new SAMRecord(header); Assert.assertNull(record.getStringAttribute(arrayTag)); record.setAttribute(arrayTag, new int[0]); From f7c46fb38fd0dce13ead6ac3c37670132dfcf8a9 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Wed, 17 Oct 2018 17:29:59 -0400 Subject: [PATCH 09/20] adding more tests and further changes --- .../java/htsjdk/samtools/TextTagCodec.java | 39 +++--- .../htsjdk/samtools/SAMRecordUnitTest.java | 32 +++-- .../htsjdk/samtools/SAMTextReaderTest.java | 20 ++++ .../htsjdk/samtools/SAMTextWriterTest.java | 14 ++- .../htsjdk/samtools/TextTagCodecTest.java | 112 ++++++++++++++++++ 5 files changed, 177 insertions(+), 40 deletions(-) create mode 100644 src/test/java/htsjdk/samtools/TextTagCodecTest.java diff --git a/src/main/java/htsjdk/samtools/TextTagCodec.java b/src/main/java/htsjdk/samtools/TextTagCodec.java index e416aa6dc0..7a9d07c07b 100644 --- a/src/main/java/htsjdk/samtools/TextTagCodec.java +++ b/src/main/java/htsjdk/samtools/TextTagCodec.java @@ -45,8 +45,11 @@ public class TextTagCodec { private static final int NUM_TAG_FIELDS = 3; private static final byte[] EMPTY_BYTE_ARRAY = new byte[0]; + private static final TagValueAndUnsignedArrayFlag EMPTY_UNSIGNED_BYTE_ARRAY = new TagValueAndUnsignedArrayFlag(EMPTY_BYTE_ARRAY, true); private static final short[] EMPTY_SHORT_ARRAY = new short[0]; + private static final TagValueAndUnsignedArrayFlag EMPTY_UNSIGNED_SHORT_ARRAY = new TagValueAndUnsignedArrayFlag(EMPTY_SHORT_ARRAY, true); private static final int[] EMPTY_INT_ARRAY = new int[0]; + private static final TagValueAndUnsignedArrayFlag EMPTY_UNSIGNED_INT_ARRAY = new TagValueAndUnsignedArrayFlag(EMPTY_INT_ARRAY, true); private static final float[] EMPTY_FLOAT_ARRAY = new float[0]; /** @@ -57,7 +60,7 @@ public class TextTagCodec { /** * Convert in-memory representation of tag to SAM text representation. * @param tagName Two-character tag name. - * @param value Tag value as approriate Object subclass. + * @param value Tag value as appropriate Object subclass. * @return SAM text String representation, i.e. name:type:value */ public String encode(final String tagName, Object value) { @@ -76,7 +79,7 @@ public String encode(final String tagName, Object value) { // H should never happen anymore. value = StringUtil.bytesToHexString((byte[])value); } else if (tagType == 'B') { - value = getArrayType(value, false) + "," + encodeArrayValue(value); + value = getArrayType(value, false) + encodeArrayValue(value); } else if (tagType == 'i') { final long longVal = ((Number) value).longValue(); // as the spec says: [-2^31, 2^32) @@ -88,7 +91,7 @@ public String encode(final String tagName, Object value) { return sb.toString(); } - private char getArrayType(final Object array, final boolean isUnsigned) { + private static char getArrayType(final Object array, final boolean isUnsigned) { final char type; final Class componentType = array.getClass().getComponentType(); if (componentType == Float.TYPE) { @@ -102,13 +105,10 @@ private char getArrayType(final Object array, final boolean isUnsigned) { return (isUnsigned? Character.toUpperCase(type): type); } - private String encodeArrayValue(final Object value) { + private static String encodeArrayValue(final Object value) { final int length = Array.getLength(value); - if (length == 0){ - return ""; - } - final StringBuilder ret = new StringBuilder(Array.get(value, 0).toString()); - for (int i = 1; i < length; ++i) { + final StringBuilder ret = new StringBuilder(); + for (int i = 0; i < length; ++i) { ret.append(','); ret.append(Array.get(value, i).toString()); } @@ -116,7 +116,7 @@ private String encodeArrayValue(final Object value) { } - private long[] widenToUnsigned(final Object array) { + private static long[] widenToUnsigned(final Object array) { final Class componentType = array.getClass().getComponentType(); final long mask; if (componentType == Byte.TYPE) mask = 0xffL; @@ -135,7 +135,7 @@ String encodeUnsignedArray(final String tagName, final Object array) { throw new IllegalArgumentException("Non-array passed to encodeUnsignedArray: " + array.getClass()); } final long[] widened = widenToUnsigned(array); - return tagName + ":B:" + getArrayType(array, true) + "," + encodeArrayValue(widened); + return tagName + ":B:" + getArrayType(array, true) + encodeArrayValue(widened); } /** @@ -183,7 +183,7 @@ public Object setValue(final Object o) { }; } - private Object convertStringToObject(final String type, final String stringVal) { + private static Object convertStringToObject(final String type, final String stringVal) { if (type.equals("Z")) { return stringVal; } else if (type.equals("A")) { @@ -227,13 +227,11 @@ else if (SAMUtils.isValidUnsignedIntegerAttribute(lValue)) { } } - private Object covertStringArrayToObject(final String stringVal) { + private static Object covertStringArrayToObject(final String stringVal) { final String[] elementTypeAndValue = new String[2]; final int numberOfTokens = StringUtil.splitConcatenateExcessTokens(stringVal, elementTypeAndValue, ','); - if (!(numberOfTokens == 1 || numberOfTokens == 2)) { - throw new SAMFormatException("Tag of type B requires an element type"); - } + if (elementTypeAndValue[0].length() != 1) { throw new SAMFormatException("Unrecognized element type for array tag value: " + elementTypeAndValue[0]); } @@ -334,14 +332,17 @@ private Object covertStringArrayToObject(final String stringVal) { private static Object createEmptyArray(char elementType) { switch ( elementType ) { case 'c': - case 'C': return EMPTY_BYTE_ARRAY; + case 'C': + return EMPTY_UNSIGNED_BYTE_ARRAY; case 's': - case 'S': return EMPTY_SHORT_ARRAY; + case 'S': + return EMPTY_UNSIGNED_SHORT_ARRAY; case 'i': - case 'I': return EMPTY_INT_ARRAY; + case 'I': + return EMPTY_UNSIGNED_INT_ARRAY; case 'f': //note that F is not a valid option since there is no signed/unsigned float distinction return EMPTY_FLOAT_ARRAY; diff --git a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java index 5bb236474b..e627e784ec 100644 --- a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java +++ b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java @@ -42,6 +42,8 @@ public class SAMRecordUnitTest extends HtsjdkTest { + private static final String ARRAY_TAG = "xa"; + @DataProvider(name = "serializationTestData") public Object[][] getSerializationTestData() { return new Object[][] { @@ -1175,17 +1177,14 @@ public void testHasAttribute(final SAMRecord samRecord, final String tag, final @Test public void test_setAttribute_empty_array() { final SAMFileHeader header = new SAMFileHeader(); - final String arrayTag = "xa"; final SAMRecord record = new SAMRecord(header); - Assert.assertNull(record.getStringAttribute(arrayTag)); - record.setAttribute(arrayTag, new int[0]); - Assert.assertNotNull(record.getSignedIntArrayAttribute(arrayTag)); - Assert.assertEquals(record.getSignedIntArrayAttribute(arrayTag),new int[0]); - record.getSAMString(); - Assert.assertEquals(record.getAttribute(arrayTag), new char[0]); - record.setAttribute(arrayTag, null); - Assert.assertNull(record.getStringAttribute(arrayTag)); - + Assert.assertNull(record.getStringAttribute(ARRAY_TAG)); + record.setAttribute(ARRAY_TAG, new int[0]); + Assert.assertNotNull(record.getSignedIntArrayAttribute(ARRAY_TAG)); + Assert.assertEquals(record.getSignedIntArrayAttribute(ARRAY_TAG), new int[0]); + Assert.assertEquals(record.getAttribute(ARRAY_TAG), new char[0]); + record.setAttribute(ARRAY_TAG, null); + Assert.assertNull(record.getStringAttribute(ARRAY_TAG)); } private static Object[][] getEmptyArrays() { @@ -1197,7 +1196,7 @@ private static Object[][] getEmptyArrays() { }; } - private static Object[][] getFileExtension(){ + private static Object[][] getFileExtensions(){ return new Object[][]{ {BamFileIoUtils.BAM_FILE_EXTENSION}, {IOUtil.SAM_FILE_EXTENSION}, {CramIO.CRAM_FILE_EXTENSION} }; @@ -1205,7 +1204,7 @@ private static Object[][] getFileExtension(){ @DataProvider public Object[][] getEmptyArraysAndExtensions(){ - return TestNGUtils.cartesianProduct(getEmptyArrays(), getFileExtension()); + return TestNGUtils.cartesianProduct(getEmptyArrays(), getFileExtensions()); } @Test(dataProvider = "getEmptyArraysAndExtensions") @@ -1213,12 +1212,11 @@ public void testWriteSamWithEmptyArray(Object emptyArray, Class arrayClass, S Assert.assertEquals(emptyArray.getClass(), arrayClass); Assert.assertEquals(Array.getLength(emptyArray), 0); - final String arrayTag = "xa"; final SAMRecordSetBuilder samRecords = new SAMRecordSetBuilder(); samRecords.addFrag("Read", 0, 100, false); final SAMRecord record = samRecords.getRecords().iterator().next(); - record.setAttribute(arrayTag, emptyArray); - checkArrayIsEmpty(arrayTag, record, arrayClass); + record.setAttribute(ARRAY_TAG, emptyArray); + checkArrayIsEmpty(ARRAY_TAG, record, arrayClass); final Path tmp = Files.createTempFile("tmp", fileExtension); IOUtil.deleteOnExit(tmp); @@ -1237,7 +1235,7 @@ public void testWriteSamWithEmptyArray(Object emptyArray, Class arrayClass, S final SAMRecordIterator iterator = reader.iterator(); Assert.assertTrue(iterator.hasNext()); final SAMRecord recordFromDisk = iterator.next(); - checkArrayIsEmpty(arrayTag, recordFromDisk, arrayClass); + checkArrayIsEmpty(ARRAY_TAG, recordFromDisk, arrayClass); } } @@ -1247,6 +1245,4 @@ private static void checkArrayIsEmpty(String arrayTag, SAMRecord recordFromDisk, Assert.assertEquals(attribute.getClass(), expectedClass); Assert.assertEquals(Array.getLength(attribute), 0); } - - } diff --git a/src/test/java/htsjdk/samtools/SAMTextReaderTest.java b/src/test/java/htsjdk/samtools/SAMTextReaderTest.java index 142eea32c1..36826d0925 100644 --- a/src/test/java/htsjdk/samtools/SAMTextReaderTest.java +++ b/src/test/java/htsjdk/samtools/SAMTextReaderTest.java @@ -27,12 +27,15 @@ import htsjdk.samtools.util.CloseableIterator; import htsjdk.samtools.util.CloserUtil; import org.testng.Assert; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; public class SAMTextReaderTest extends HtsjdkTest { + private static final String ARRAY_TAG = "xa"; + // Simple input, spot check that parsed correctly, and make sure nothing blows up. @Test public void testBasic() throws Exception { @@ -135,4 +138,21 @@ public void testTagWithColon() { Assert.assertEquals(recFromText.getAttribute(SAMTag.CQ.name()), valueWithColons); CloserUtil.close(reader); } + + @DataProvider + public Object[][] getRecordsWithArrays(){ + final String recordBase = "Read\t4\tchr1\t1\t0\t*\t*\t0\t0\tG\t%\t"; + return new Object[][]{ + {recordBase + ARRAY_TAG + ":B:i", new int[0]}, + {recordBase + ARRAY_TAG + ":B:i,", new int[0]}, + {recordBase + ARRAY_TAG + ":B:i,1,2,3,", new int[]{1,2,3}}, + }; + } + + @Test(dataProvider = "getRecordsWithArrays") + public void testSamRecordCanHandleArrays(String samRecord, Object array){ + final SAMLineParser samLineParser = new SAMLineParser(new SAMFileHeader()); + final SAMRecord record = samLineParser.parseLine(samRecord); + Assert.assertEquals(record.getAttribute(ARRAY_TAG), array); + } } diff --git a/src/test/java/htsjdk/samtools/SAMTextWriterTest.java b/src/test/java/htsjdk/samtools/SAMTextWriterTest.java index 5c9ff28cde..959d5818f1 100644 --- a/src/test/java/htsjdk/samtools/SAMTextWriterTest.java +++ b/src/test/java/htsjdk/samtools/SAMTextWriterTest.java @@ -34,7 +34,7 @@ public class SAMTextWriterTest extends HtsjdkTest { - private SAMRecordSetBuilder getSAMReader(final boolean sortForMe, final SAMFileHeader.SortOrder sortOrder) { + private SAMRecordSetBuilder getSamRecordSet(final boolean sortForMe, final SAMFileHeader.SortOrder sortOrder) { final SAMRecordSetBuilder ret = new SAMRecordSetBuilder(sortForMe, sortOrder); ret.addPair("readB", 20, 200, 300); ret.addPair("readA", 20, 100, 150); @@ -45,7 +45,7 @@ private SAMRecordSetBuilder getSAMReader(final boolean sortForMe, final SAMFileH @Test public void testNullHeader() throws Exception { - final SAMRecordSetBuilder recordSetBuilder = getSAMReader(true, SAMFileHeader.SortOrder.coordinate); + final SAMRecordSetBuilder recordSetBuilder = getSamRecordSet(true, SAMFileHeader.SortOrder.coordinate); for (final SAMRecord rec : recordSetBuilder.getRecords()) { rec.setHeader(null); } @@ -77,7 +77,7 @@ private void doTest(final SAMRecordSetBuilder recordSetBuilder) throws Exception } private void doTest(final SamFlagField samFlagField) throws Exception { - doTest(getSAMReader(true, SAMFileHeader.SortOrder.coordinate), samFlagField); + doTest(getSamRecordSet(true, SAMFileHeader.SortOrder.coordinate), samFlagField); } private void doTest(final SAMRecordSetBuilder recordSetBuilder, final SamFlagField samFlagField) throws Exception { @@ -128,4 +128,12 @@ private void doTest(final SAMRecordSetBuilder recordSetBuilder, final SamFlagFie Assert.assertFalse(newSAMIt.hasNext()); inputSAM.close(); } + + @Test + public void testEmptyArrayAttributeHasNoCommaWhenWrittenToSAM(){ + final SAMFileHeader header = new SAMFileHeader(); + final SAMRecord record = new SAMRecord(header); + record.setAttribute("xa", new int[0]); + Assert.assertTrue(record.getSAMString().endsWith("xa:B:i")); + } } diff --git a/src/test/java/htsjdk/samtools/TextTagCodecTest.java b/src/test/java/htsjdk/samtools/TextTagCodecTest.java new file mode 100644 index 0000000000..83cda20b16 --- /dev/null +++ b/src/test/java/htsjdk/samtools/TextTagCodecTest.java @@ -0,0 +1,112 @@ +package htsjdk.samtools; + +import org.testng.Assert; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import java.util.Map; + +public class TextTagCodecTest { + + @DataProvider + public Object[][] getArraysToEncode(){ + return new Object[][]{ + {new byte[0], true, "xa:B:c"}, + {new byte[0], false, "xa:B:C"}, + {new byte[]{1, 2, 3}, true, "xa:B:c,1,2,3"}, + {new byte[]{1, 2, 3}, false, "xa:B:C,1,2,3"}, + + {new short[0], true, "xa:B:s"}, + {new short[0], false, "xa:B:S"}, + {new short[]{1, 2, 3}, true, "xa:B:s,1,2,3"}, + {new short[]{1, 2, 3}, false, "xa:B:S,1,2,3"}, + + {new int[0], true, "xa:B:i"}, + {new int[0], false, "xa:B:I"}, + {new int[]{1, 2, 3}, true, "xa:B:i,1,2,3"}, + {new int[]{1, 2, 3}, false, "xa:B:I,1,2,3"}, + + {new float[0], true, "xa:B:f"}, + {new float[]{1.0f, 2.0f, 3.0f}, true, "xa:B:f,1.0,2.0,3.0"}, + + }; + } + + @Test(dataProvider = "getArraysToEncode") + public void testEmptyAndNonEmptyArrayEncoding(Object array, boolean isSigned, String expectedTag){ + final TextTagCodec textTagCodec = new TextTagCodec(); + final String tagName = "xa"; + final String encodedTag = isSigned + ? textTagCodec.encode(tagName, array) + : textTagCodec.encodeUnsignedArray(tagName, array); + Assert.assertEquals(encodedTag, expectedTag); + + } + + @DataProvider + public Object[][] getArraysToDecode(){ + return new Object[][]{ + {"xa:B:c", new byte[0]}, + {"xa:B:C", new TagValueAndUnsignedArrayFlag(new byte[0], true)}, + {"xa:B:c,1,2,3", new byte[]{1, 2, 3}}, + {"xa:B:C,1,2,3", new TagValueAndUnsignedArrayFlag(new byte[]{1, 2, 3}, true)}, + {"xa:B:c,", new byte[0]}, + {"xa:B:C,", new TagValueAndUnsignedArrayFlag(new byte[0], true)}, + + {"xa:B:s", new short[0]}, + {"xa:B:S", new TagValueAndUnsignedArrayFlag(new short[0], true)}, + {"xa:B:s,1,2,3", new short[]{1, 2, 3}}, + {"xa:B:S,1,2,3", new TagValueAndUnsignedArrayFlag(new short[]{1, 2, 3}, true)}, + {"xa:B:s,", new short[0]}, + {"xa:B:S,", new TagValueAndUnsignedArrayFlag(new short[0], true)}, + + {"xa:B:i", new int[0]}, + {"xa:B:I", new TagValueAndUnsignedArrayFlag(new int[0], true)}, + {"xa:B:i,1,2,3", new int[]{1, 2, 3}}, + {"xa:B:I,1,2,3", new TagValueAndUnsignedArrayFlag(new int[]{1, 2, 3}, true)}, + {"xa:B:i,", new int[0]}, + {"xa:B:I,", new TagValueAndUnsignedArrayFlag(new int[0], true)}, + + {"xa:B:f", new float[0]}, + {"xa:B:f,", new float[0]}, + {"xa:B:f,1.0,2.0,3.0", new float[]{1.0f, 2.0f, 3.0f}}, + }; + } + + @Test(dataProvider = "getArraysToDecode") + public void testEmptyAndNonEmptyArrayDecoding(String tag, Object expectedValue) { + final TextTagCodec textTagCodec = new TextTagCodec(); + final Map.Entry decoded = textTagCodec.decode(tag); + + Assert.assertEquals(decoded.getKey(), "xa"); + + final Object value = decoded.getValue(); + if( value instanceof TagValueAndUnsignedArrayFlag){ + Assert.assertTrue(expectedValue instanceof TagValueAndUnsignedArrayFlag); + final TagValueAndUnsignedArrayFlag typedValue = (TagValueAndUnsignedArrayFlag) value; + final TagValueAndUnsignedArrayFlag typedExpected = (TagValueAndUnsignedArrayFlag) expectedValue; + Assert.assertEquals(typedValue.value, typedExpected.value); + Assert.assertEquals(typedValue.isUnsignedArray, typedExpected.isUnsignedArray); + } else { + Assert.assertEquals(value, expectedValue); + } + } + + @DataProvider + public Object[][] getBadArrayTags(){ + return new Object[][]{ + {"xa:B"}, // no colon + {"xa:B:F"}, // there is no such thing as unsigned floating point + {"xa:B:F,1.0,2.0"}, // same as above but empty arrays have a special path + {"xa:B:,1,2,3"}, // missing type + {"xa:B:c,700"}, //out of bounds + {"xa:B:C,-10"}, // negative in unsigned array + }; + } + + @Test(dataProvider = "getBadArrayTags", expectedExceptions = SAMFormatException.class) + public void testBadArrayTags(String badTag){ + final TextTagCodec textTagCodec = new TextTagCodec(); + textTagCodec.decode(badTag); + } +} \ No newline at end of file From ebf496fbb208562af97fa911bb22702841be2dcb Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Wed, 17 Oct 2018 17:31:58 -0400 Subject: [PATCH 10/20] fix space --- src/test/java/htsjdk/samtools/SAMRecordUnitTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java index e627e784ec..decf011792 100644 --- a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java +++ b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java @@ -1225,11 +1225,11 @@ public void testWriteSamWithEmptyArray(Object emptyArray, Class arrayClass, S .setCreateMd5File(false) .setCreateIndex(false); final Path reference = IOUtil.getPath("src/test/resources/htsjdk/samtools/one-contig.fasta"); - try(final SAMFileWriter samFileWriter = writerFactory.makeWriter(samRecords.getHeader(), false, tmp, reference)) { + try (final SAMFileWriter samFileWriter = writerFactory.makeWriter(samRecords.getHeader(), false, tmp, reference)) { samFileWriter.addAlignment(record); } - try(final SamReader reader = SamReaderFactory.makeDefault() + try (final SamReader reader = SamReaderFactory.makeDefault() .referenceSequence(reference) .open(tmp)) { final SAMRecordIterator iterator = reader.iterator(); From 9b7476feba1c5c3ccf9f03e5680898a87c3e1845 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Wed, 17 Oct 2018 18:10:13 -0400 Subject: [PATCH 11/20] hopefully tests pass... --- src/test/java/htsjdk/samtools/SAMTextWriterTest.java | 2 +- src/test/java/htsjdk/samtools/TextTagCodecTest.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/java/htsjdk/samtools/SAMTextWriterTest.java b/src/test/java/htsjdk/samtools/SAMTextWriterTest.java index 959d5818f1..1ee916f58f 100644 --- a/src/test/java/htsjdk/samtools/SAMTextWriterTest.java +++ b/src/test/java/htsjdk/samtools/SAMTextWriterTest.java @@ -134,6 +134,6 @@ public void testEmptyArrayAttributeHasNoCommaWhenWrittenToSAM(){ final SAMFileHeader header = new SAMFileHeader(); final SAMRecord record = new SAMRecord(header); record.setAttribute("xa", new int[0]); - Assert.assertTrue(record.getSAMString().endsWith("xa:B:i")); + Assert.assertTrue(record.getSAMString().endsWith("xa:B:i\n")); } } diff --git a/src/test/java/htsjdk/samtools/TextTagCodecTest.java b/src/test/java/htsjdk/samtools/TextTagCodecTest.java index 83cda20b16..7b80b8d7ab 100644 --- a/src/test/java/htsjdk/samtools/TextTagCodecTest.java +++ b/src/test/java/htsjdk/samtools/TextTagCodecTest.java @@ -1,12 +1,13 @@ package htsjdk.samtools; +import htsjdk.HtsjdkTest; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.util.Map; -public class TextTagCodecTest { +public class TextTagCodecTest extends HtsjdkTest { @DataProvider public Object[][] getArraysToEncode(){ From 37630ebb7e9b4fca6d30705ba87a453d8002455d Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Thu, 18 Oct 2018 11:19:32 -0400 Subject: [PATCH 12/20] more htsjdktest --- .../utils/{TestNGUtilsUnitTest.java => TestNGUtilsTest.java} | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) rename src/test/java/htsjdk/utils/{TestNGUtilsUnitTest.java => TestNGUtilsTest.java} (97%) diff --git a/src/test/java/htsjdk/utils/TestNGUtilsUnitTest.java b/src/test/java/htsjdk/utils/TestNGUtilsTest.java similarity index 97% rename from src/test/java/htsjdk/utils/TestNGUtilsUnitTest.java rename to src/test/java/htsjdk/utils/TestNGUtilsTest.java index e1718e014f..479cd4ff33 100644 --- a/src/test/java/htsjdk/utils/TestNGUtilsUnitTest.java +++ b/src/test/java/htsjdk/utils/TestNGUtilsTest.java @@ -1,5 +1,6 @@ package htsjdk.utils; +import htsjdk.HtsjdkTest; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -7,7 +8,7 @@ import java.util.Arrays; import java.util.List; -public class TestNGUtilsUnitTest { +public class TestNGUtilsTest extends HtsjdkTest { @DataProvider public Object[][] getArraysAndLists() { From 98ceb538e3d98689c8d22997d3a3253308c370fa Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Thu, 18 Oct 2018 14:32:25 -0400 Subject: [PATCH 13/20] Update src/main/java/htsjdk/samtools/TextTagCodec.java --- src/main/java/htsjdk/samtools/TextTagCodec.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/htsjdk/samtools/TextTagCodec.java b/src/main/java/htsjdk/samtools/TextTagCodec.java index 7a9d07c07b..107b1cb041 100644 --- a/src/main/java/htsjdk/samtools/TextTagCodec.java +++ b/src/main/java/htsjdk/samtools/TextTagCodec.java @@ -344,7 +344,7 @@ private static Object createEmptyArray(char elementType) { case 'I': return EMPTY_UNSIGNED_INT_ARRAY; case 'f': - //note that F is not a valid option since there is no signed/unsigned float distinction + // Note that F is not a valid option since there is no signed/unsigned float distinction. return EMPTY_FLOAT_ARRAY; default: { throw new SAMFormatException("Unrecognized array tag element type: " + elementType); } } From 5d591c3e0dde01e9888333c875f7116f86b7929b Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Thu, 18 Oct 2018 14:32:32 -0400 Subject: [PATCH 14/20] Update src/main/java/htsjdk/samtools/TextTagCodec.java --- src/main/java/htsjdk/samtools/TextTagCodec.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/htsjdk/samtools/TextTagCodec.java b/src/main/java/htsjdk/samtools/TextTagCodec.java index 107b1cb041..d3c5ba5aec 100644 --- a/src/main/java/htsjdk/samtools/TextTagCodec.java +++ b/src/main/java/htsjdk/samtools/TextTagCodec.java @@ -330,7 +330,7 @@ private static Object covertStringArrayToObject(final String stringVal) { } private static Object createEmptyArray(char elementType) { - switch ( elementType ) { + switch (elementType) { case 'c': return EMPTY_BYTE_ARRAY; case 'C': From 1d41109d56531f894dabfd0abcbe10484cedba52 Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Thu, 18 Oct 2018 14:32:45 -0400 Subject: [PATCH 15/20] Update src/main/java/htsjdk/samtools/TextTagCodec.java --- src/main/java/htsjdk/samtools/TextTagCodec.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/htsjdk/samtools/TextTagCodec.java b/src/main/java/htsjdk/samtools/TextTagCodec.java index d3c5ba5aec..d4f5ceecd5 100644 --- a/src/main/java/htsjdk/samtools/TextTagCodec.java +++ b/src/main/java/htsjdk/samtools/TextTagCodec.java @@ -346,7 +346,8 @@ private static Object createEmptyArray(char elementType) { case 'f': // Note that F is not a valid option since there is no signed/unsigned float distinction. return EMPTY_FLOAT_ARRAY; - default: { throw new SAMFormatException("Unrecognized array tag element type: " + elementType); } + default: + throw new SAMFormatException("Unrecognized array tag element type: " + elementType); } } From 2306e3f3669ff4dfe20f01405a0d995ed8d7ff18 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Thu, 18 Oct 2018 14:34:51 -0400 Subject: [PATCH 16/20] more comments --- src/test/java/htsjdk/utils/TestNGUtils.java | 17 ++++----- .../java/htsjdk/utils/TestNGUtilsTest.java | 36 +++++++++---------- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/src/test/java/htsjdk/utils/TestNGUtils.java b/src/test/java/htsjdk/utils/TestNGUtils.java index 4bcdecf3c8..096bd4eb0c 100644 --- a/src/test/java/htsjdk/utils/TestNGUtils.java +++ b/src/test/java/htsjdk/utils/TestNGUtils.java @@ -46,19 +46,16 @@ public static Iterator getDataProviders(final String packageName) { /** * @return the cartesian product of two or more DataProviders than can be used as a new dataprovider */ - public static Object[][] cartesianProduct(Object[][] ... dataProviders){ - final List[] lists = Arrays.stream(dataProviders) + public static Object[][] cartesianProduct(Object[][]... dataProviders) { + List>> lists = Arrays.stream(dataProviders) .map(TestNGUtils::nestedArraysToNestedLists) - .toArray(List[]::new); - - @SuppressWarnings("unchecked") + .collect(Collectors.toList()); final List>> product = Lists.cartesianProduct(lists); final List> mergeProduct = product.stream() - .map((List> list) -> { - List result = new ArrayList<>(); - list.forEach(result::addAll); - return result; - }).collect(Collectors.toList()); + .map( l -> l.stream() + .flatMap(Collection::stream) + .collect(Collectors.toList())) + .collect(Collectors.toList()); return nestedListsToNestedArrays(mergeProduct); } diff --git a/src/test/java/htsjdk/utils/TestNGUtilsTest.java b/src/test/java/htsjdk/utils/TestNGUtilsTest.java index 479cd4ff33..7a082687e5 100644 --- a/src/test/java/htsjdk/utils/TestNGUtilsTest.java +++ b/src/test/java/htsjdk/utils/TestNGUtilsTest.java @@ -17,11 +17,11 @@ public Object[][] getArraysAndLists() { new Object[][]{ {1, 2}, {3, 4} - } - , Arrays.asList( - Arrays.asList(1, 2), - Arrays.asList(3, 4) - ) + }, + Arrays.asList( + Arrays.asList(1, 2), + Arrays.asList(3, 4) + ) }, { new Object[][]{ @@ -33,12 +33,12 @@ public Object[][] getArraysAndLists() { }, { new Object[][]{ - {1,2,3}, - {4,5,6} + {1, 2, 3}, + {4, 5, 6} }, Arrays.asList( - Arrays.asList(1,2,3), - Arrays.asList(4,5,6) + Arrays.asList(1, 2, 3), + Arrays.asList(4, 5, 6) ) }, { @@ -59,28 +59,28 @@ public Object[][] getArraysAndLists() { } @Test(dataProvider = "getArraysAndLists") - public void testObjectsToLists(Object[][] objects, List> lists){ + public void testObjectsToLists(Object[][] objects, List> lists) { Assert.assertEquals(TestNGUtils.nestedArraysToNestedLists(objects), lists); } @Test(dataProvider = "getArraysAndLists") - public void testListsToArrays(Object[][] objects, List> lists){ + public void testListsToArrays(Object[][] objects, List> lists) { final Object[][] convertedObjects = TestNGUtils.nestedListsToNestedArrays(lists); assertNestedArraysEqual(objects, convertedObjects); } private static void assertNestedArraysEqual(Object[][] objects, Object[][] convertedObjects) { - for(int i = 0; i < objects.length; i++){ + for (int i = 0; i < objects.length; i++) { Assert.assertEquals(convertedObjects[i], objects[i]); } } @DataProvider - public Object[][] getDataProviders(){ + public Object[][] getDataProviders() { return new Object[][]{ - { new Object[][]{ {1,2} }, new Object[][]{{3},{ 4}}, new Object[][]{{1, 2, 3}, {1, 2, 4}}}, - { new Object[][]{ {1} }, new Object[][]{{2}}, new Object[][]{{1,2}}}, - { new Object[][]{ {1},{2},{3} }, new Object[][]{ {'a', 'b'}, {'c','d'} }, + {new Object[][]{{1, 2}}, new Object[][]{{3}, {4}}, new Object[][]{{1, 2, 3}, {1, 2, 4}}}, + {new Object[][]{{1}}, new Object[][]{{2}}, new Object[][]{{1, 2}}}, + {new Object[][]{{1}, {2}, {3}}, new Object[][]{{'a', 'b'}, {'c', 'd'}}, new Object[][]{{1, 'a', 'b'}, {1, 'c', 'd'}, {2, 'a', 'b'}, {2, 'c', 'd'}, {3, 'a', 'b'}, {3, 'c', 'd'}}} @@ -88,8 +88,8 @@ public Object[][] getDataProviders(){ } @Test(dataProvider = "getDataProviders") - public void testProduct(Object[][] dataProvider1, Object[][] dataProvider2, Object[][] expectedResult){ + public void testProduct(Object[][] dataProvider1, Object[][] dataProvider2, Object[][] expectedResult) { final Object[][] actual = TestNGUtils.cartesianProduct(dataProvider1, dataProvider2); - assertNestedArraysEqual( actual, expectedResult); + assertNestedArraysEqual(actual, expectedResult); } } From fbfdd7981eaa1bbb2bd614c46fb6231d74545d19 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Mon, 22 Oct 2018 14:56:31 -0400 Subject: [PATCH 17/20] working on comments --- .../java/htsjdk/samtools/TextTagCodec.java | 36 ++----------------- .../java/htsjdk/utils/TestNGUtilsTest.java | 5 +-- 2 files changed, 5 insertions(+), 36 deletions(-) diff --git a/src/main/java/htsjdk/samtools/TextTagCodec.java b/src/main/java/htsjdk/samtools/TextTagCodec.java index d4f5ceecd5..e86bef0412 100644 --- a/src/main/java/htsjdk/samtools/TextTagCodec.java +++ b/src/main/java/htsjdk/samtools/TextTagCodec.java @@ -44,13 +44,7 @@ public class TextTagCodec { // 3 fields for non-empty strings 2 fields if the string is empty. private static final int NUM_TAG_FIELDS = 3; - private static final byte[] EMPTY_BYTE_ARRAY = new byte[0]; - private static final TagValueAndUnsignedArrayFlag EMPTY_UNSIGNED_BYTE_ARRAY = new TagValueAndUnsignedArrayFlag(EMPTY_BYTE_ARRAY, true); - private static final short[] EMPTY_SHORT_ARRAY = new short[0]; - private static final TagValueAndUnsignedArrayFlag EMPTY_UNSIGNED_SHORT_ARRAY = new TagValueAndUnsignedArrayFlag(EMPTY_SHORT_ARRAY, true); - private static final int[] EMPTY_INT_ARRAY = new int[0]; - private static final TagValueAndUnsignedArrayFlag EMPTY_UNSIGNED_INT_ARRAY = new TagValueAndUnsignedArrayFlag(EMPTY_INT_ARRAY, true); - private static final float[] EMPTY_FLOAT_ARRAY = new float[0]; + private static final String[] EMPTY_STRING_ARRAY = new String[0]; /** * This is really a local variable of decode(), but allocated here to reduce allocations. @@ -237,12 +231,8 @@ private static Object covertStringArrayToObject(final String stringVal) { } final char elementType = elementTypeAndValue[0].charAt(0); - if (numberOfTokens == 1) { - return createEmptyArray(elementType); - } - final String[] stringValues = elementTypeAndValue[1].split(","); - if (stringValues.length == 0) throw new SAMFormatException("Tag of type B should have at least one element"); + final String[] stringValues = elementTypeAndValue[1] != null ? elementTypeAndValue[1].split(",") : EMPTY_STRING_ARRAY; if (elementType == 'f') { final float[] ret = new float[stringValues.length]; for (int i = 0; i < stringValues.length; ++i) { @@ -329,28 +319,6 @@ private static Object covertStringArrayToObject(final String stringVal) { } } - private static Object createEmptyArray(char elementType) { - switch (elementType) { - case 'c': - return EMPTY_BYTE_ARRAY; - case 'C': - return EMPTY_UNSIGNED_BYTE_ARRAY; - case 's': - return EMPTY_SHORT_ARRAY; - case 'S': - return EMPTY_UNSIGNED_SHORT_ARRAY; - case 'i': - return EMPTY_INT_ARRAY; - case 'I': - return EMPTY_UNSIGNED_INT_ARRAY; - case 'f': - // Note that F is not a valid option since there is no signed/unsigned float distinction. - return EMPTY_FLOAT_ARRAY; - default: - throw new SAMFormatException("Unrecognized array tag element type: " + elementType); - } - } - Iso8601Date decodeDate(final String dateStr) { try { return new Iso8601Date(dateStr); diff --git a/src/test/java/htsjdk/utils/TestNGUtilsTest.java b/src/test/java/htsjdk/utils/TestNGUtilsTest.java index 7a082687e5..9eb43aaaaf 100644 --- a/src/test/java/htsjdk/utils/TestNGUtilsTest.java +++ b/src/test/java/htsjdk/utils/TestNGUtilsTest.java @@ -54,7 +54,7 @@ public Object[][] getArraysAndLists() { Arrays.asList(3), Arrays.asList(4) ) - } + }, }; } @@ -83,7 +83,8 @@ public Object[][] getDataProviders() { {new Object[][]{{1}, {2}, {3}}, new Object[][]{{'a', 'b'}, {'c', 'd'}}, new Object[][]{{1, 'a', 'b'}, {1, 'c', 'd'}, {2, 'a', 'b'}, {2, 'c', 'd'}, - {3, 'a', 'b'}, {3, 'c', 'd'}}} + {3, 'a', 'b'}, {3, 'c', 'd'}}}, + {new Object[][]{ new Object[0], new Object[0], new Object[0]}} }; } From c1e19ec3a5ff344cfe1a9794c8c9dc670095a456 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Mon, 22 Oct 2018 17:02:58 -0400 Subject: [PATCH 18/20] more test --- .../htsjdk/samtools/TextTagCodecTest.java | 1 - src/test/java/htsjdk/tribble/TestUtils.java | 2 - src/test/java/htsjdk/utils/TestNGUtils.java | 4 ++ .../java/htsjdk/utils/TestNGUtilsTest.java | 57 ++++++++++++++++++- 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/test/java/htsjdk/samtools/TextTagCodecTest.java b/src/test/java/htsjdk/samtools/TextTagCodecTest.java index 7b80b8d7ab..7cedf19f34 100644 --- a/src/test/java/htsjdk/samtools/TextTagCodecTest.java +++ b/src/test/java/htsjdk/samtools/TextTagCodecTest.java @@ -41,7 +41,6 @@ public void testEmptyAndNonEmptyArrayEncoding(Object array, boolean isSigned, St ? textTagCodec.encode(tagName, array) : textTagCodec.encodeUnsignedArray(tagName, array); Assert.assertEquals(encodedTag, expectedTag); - } @DataProvider diff --git a/src/test/java/htsjdk/tribble/TestUtils.java b/src/test/java/htsjdk/tribble/TestUtils.java index d958e14306..c77f58a68e 100644 --- a/src/test/java/htsjdk/tribble/TestUtils.java +++ b/src/test/java/htsjdk/tribble/TestUtils.java @@ -64,6 +64,4 @@ public static Path getTribbleFileInJimfs(String vcf, String index, FileSystem fi } return Files.copy(vcfPath, vcfDestination); } - - } diff --git a/src/test/java/htsjdk/utils/TestNGUtils.java b/src/test/java/htsjdk/utils/TestNGUtils.java index 096bd4eb0c..ed11f1ac92 100644 --- a/src/test/java/htsjdk/utils/TestNGUtils.java +++ b/src/test/java/htsjdk/utils/TestNGUtils.java @@ -44,6 +44,10 @@ public static Iterator getDataProviders(final String packageName) { } /** + * Combine two or more Dataproviders by taking the cartesian product of the their test cases. + * + * Note: In the case of a an empty provider, the result will be the product of the non-empty providers. + * This isdifferent from the traditional definition of the cartesian product. * @return the cartesian product of two or more DataProviders than can be used as a new dataprovider */ public static Object[][] cartesianProduct(Object[][]... dataProviders) { diff --git a/src/test/java/htsjdk/utils/TestNGUtilsTest.java b/src/test/java/htsjdk/utils/TestNGUtilsTest.java index 9eb43aaaaf..63dfc59973 100644 --- a/src/test/java/htsjdk/utils/TestNGUtilsTest.java +++ b/src/test/java/htsjdk/utils/TestNGUtilsTest.java @@ -6,10 +6,13 @@ import org.testng.annotations.Test; import java.util.Arrays; +import java.util.Collections; import java.util.List; public class TestNGUtilsTest extends HtsjdkTest { + public static final Object[] EMPTY_ARRAY = new Object[0]; + @DataProvider public Object[][] getArraysAndLists() { return new Object[][]{ @@ -80,11 +83,19 @@ public Object[][] getDataProviders() { return new Object[][]{ {new Object[][]{{1, 2}}, new Object[][]{{3}, {4}}, new Object[][]{{1, 2, 3}, {1, 2, 4}}}, {new Object[][]{{1}}, new Object[][]{{2}}, new Object[][]{{1, 2}}}, + {new Object[][]{{1}}, new Object[][]{{2, 3}}, new Object[][]{{1, 2, 3}}}, {new Object[][]{{1}, {2}, {3}}, new Object[][]{{'a', 'b'}, {'c', 'd'}}, new Object[][]{{1, 'a', 'b'}, {1, 'c', 'd'}, {2, 'a', 'b'}, {2, 'c', 'd'}, {3, 'a', 'b'}, {3, 'c', 'd'}}}, - {new Object[][]{ new Object[0], new Object[0], new Object[0]}} + {new Object[][]{{}}, new Object[][]{{}}, new Object[][]{{}}}, + {new Object[][]{{}}, new Object[][]{{1}}, new Object[][]{{1}}}, + {new Object[][]{{}}, new Object[][]{{1, 2}}, new Object[][]{{1, 2}}}, + {new Object[][]{{1}}, new Object[][]{{}}, new Object[][]{{1}}}, + {new Object[][]{{}}, new Object[][]{{1}}, new Object[][]{{1}}}, + {new Object[][]{{EMPTY_ARRAY}}, new Object[][]{{1}}, new Object[][]{{EMPTY_ARRAY, 1}}}, + {new Object[][]{{1}, {2}, {3}}, new Object[][]{{4}, {5}, {6}}, + new Object[][]{{1,4}, {1,5}, {1,6}, {2, 4}, {2, 5}, {2, 6}, {3, 4}, {3, 5}, {3, 6}}} }; } @@ -93,4 +104,48 @@ public void testProduct(Object[][] dataProvider1, Object[][] dataProvider2, Obje final Object[][] actual = TestNGUtils.cartesianProduct(dataProvider1, dataProvider2); assertNestedArraysEqual(actual, expectedResult); } + + + @DataProvider + public Object[][] getDifferingNumbersOfProviders() { + final Object[][] p1 = new Object[][]{{1}, {2}}; + final Object[][] p2 = new Object[][]{{"a", "b"}}; + final Object[][] p3 = new Object[][]{{}}; + final Object[][] p4 = new Object[][]{{3}, {4}, {5}}; + + final Object[][] expected0 = new Object[][]{{}}; + final Object[][] expected1 = new Object[][]{{1}, {2}}; + final Object[][] expected2 = new Object[][]{{1, "a", "b"}, {2, "a", "b"}}; + final Object[][] expected3 = expected2; + final Object[][] expected4 = new Object[][]{ + {1, "a", "b", 3}, + {1, "a", "b", 4}, + {1, "a", "b", 5}, + {2, "a", "b", 3}, + {2, "a", "b", 4}, + {2, "a", "b", 5} + }; + + return new Object[][]{ + {Arrays.asList(), expected0}, + {Collections.singletonList(p1), expected1}, + {Arrays.asList(p1, p2), expected2}, + {Arrays.asList(p1, p2, p3), expected3}, + {Arrays.asList(p1, p2, p3, p4), expected4} + }; + } + + @Test(dataProvider = "getDifferingNumbersOfProviders") + public void testCartesianProductOfManyProviders(List providers, Object[][] expected){ + final Object[][] product = TestNGUtils.cartesianProduct(providers.toArray(new Object[][][]{})); + assertNestedArraysEqual(product, expected); + } + + @Test + public void testSingleProvider() { + final Object[][] expected = {{1, 2}}; + final Object[][] product = TestNGUtils.cartesianProduct(expected); + assertNestedArraysEqual(product, expected); + } + } From 8bd22589fa03a9d439870830fb2f0d3d8790978c Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Wed, 24 Oct 2018 16:48:58 -0400 Subject: [PATCH 19/20] Update src/test/java/htsjdk/utils/TestNGUtils.java --- src/test/java/htsjdk/utils/TestNGUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/htsjdk/utils/TestNGUtils.java b/src/test/java/htsjdk/utils/TestNGUtils.java index ed11f1ac92..3eed34a788 100644 --- a/src/test/java/htsjdk/utils/TestNGUtils.java +++ b/src/test/java/htsjdk/utils/TestNGUtils.java @@ -47,7 +47,7 @@ public static Iterator getDataProviders(final String packageName) { * Combine two or more Dataproviders by taking the cartesian product of the their test cases. * * Note: In the case of a an empty provider, the result will be the product of the non-empty providers. - * This isdifferent from the traditional definition of the cartesian product. + * This is different from the traditional definition of the cartesian product. * @return the cartesian product of two or more DataProviders than can be used as a new dataprovider */ public static Object[][] cartesianProduct(Object[][]... dataProviders) { From 31cc9acc5232c74ce0e87c1c68e31d0566d7e0f5 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Wed, 24 Oct 2018 16:49:18 -0400 Subject: [PATCH 20/20] Update src/test/java/htsjdk/utils/TestNGUtilsTest.java --- src/test/java/htsjdk/utils/TestNGUtilsTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/htsjdk/utils/TestNGUtilsTest.java b/src/test/java/htsjdk/utils/TestNGUtilsTest.java index 63dfc59973..9fb811a7db 100644 --- a/src/test/java/htsjdk/utils/TestNGUtilsTest.java +++ b/src/test/java/htsjdk/utils/TestNGUtilsTest.java @@ -147,5 +147,4 @@ public void testSingleProvider() { final Object[][] product = TestNGUtils.cartesianProduct(expected); assertNestedArraysEqual(product, expected); } - }