README.md

@@ -49,12 +49,9 @@ Example gradle usage from the htsjdk root directory:
  ```
  ./gradlew test
 
- ./gradlew test -Dtest.single=TestClassName
+ ./gradlew test --tests AlleleUnitTest
 
- ./gradlew test --tests htsjdk.variant.variantcontext.AlleleUnitTest
- ./gradlew test --tests "*AlleleUnitTest"
-
- ./gradlew test --tests "*AlleleUnitTest" --debug-jvm
+ ./gradlew test --tests AlleleUnitTest --debug-jvm
  ```
 
 - run tests and collect coverage information (report will be in `build/reports/jacoco/test/html/index.html`)

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

debian/changelog

+htsjdk (2.18.0+dfsg-1) UNRELEASED; urgency=medium
+
+  * New upstream version
+  TODO: Fix or drop failing tests
+
+ -- Andreas Tille <tille@debian.org>  Sun, 02 Dec 2018 11:18:29 +0100
+
 htsjdk (2.16.1+dfsg-3) unstable; urgency=medium
 
   * Fix FTBS on debian test servers due to ip/name resolution

debian/patches/100-fix-java11

@@ -6,22 +6,23 @@ Last-Updated: 2018-11-12
 Forwarded: no
 --- a/build.gradle
 +++ b/build.gradle
-@@ -22,12 +22,14 @@
+@@ -22,6 +22,7 @@ dependencies {
      compile "org.apache.commons:commons-compress:1.4.1"
      compile "org.tukaani:xz:1.5"
      compile "gov.nih.nlm.ncbi:ngs-java:2.9.0"
 +    compile "javax.xml.bind:jaxb-api:debian"
  
-     testCompile "org.scala-lang:scala-library:2.11.x"
+     testCompile "org.scala-lang:scala-library:2.12.x"
      //testCompile "org.scalatest:scalatest_2.12:3.0.5"
-     //testRuntime 'org.pegdown:pegdown:1.6.0' // Necessary for generating HTML reports with ScalaTest
+@@ -29,6 +30,7 @@ dependencies {
      testCompile "org.testng:testng:debian"
      testCompile "com.google.jimfs:jimfs:debian"
+     testCompile "com.google.guava:guava:debian"
 +    testCompile "com.sun.xml.bind:jaxb-impl:debian"
   }
  
  sourceCompatibility = 1.8
-@@ -93,7 +95,6 @@
+@@ -94,7 +96,6 @@ test {
      if (System.env.CI == "true") {
          jvmArgs += '-Dsamjdk.sra_libraries_download=true'
      }
@@ -29,7 +30,7 @@ Forwarded: no
  
      exclude 'htsjdk/samtools/sra/**'
      exclude 'htsjdk/samtools/BAMFileIndexTest.java'
-@@ -120,7 +121,6 @@
+@@ -121,7 +122,6 @@ wrapper {
  if (JavaVersion.current().isJava8Compatible()) {
      tasks.withType(Javadoc) {
          options.addStringOption('Xdoclint:none', '-quiet')

debian/patches/13-skip_network_tests

@@ -5,7 +5,7 @@ Description: remove network related tests
 Last-Updated: 2015-01-06
 --- a/src/test/java/htsjdk/samtools/seekablestream/SeekableBufferedStreamTest.java
 +++ b/src/test/java/htsjdk/samtools/seekablestream/SeekableBufferedStreamTest.java
-@@ -42,58 +42,7 @@
+@@ -42,72 +42,6 @@ public class SeekableBufferedStreamTest
      private final String BAM_URL_STRING = "http://broadinstitute.github.io/picard/testdata/index_test.bam";
      private static File TestFile = new File("src/test/resources/htsjdk/samtools/seekablestream/megabyteZeros.dat");
  
@@ -36,6 +36,20 @@ Last-Updated: 2015-01-06
 -        assertEquals(buffer1, buffer2);
 -    }
 -
+-    @Test
+-    public void testReadExactlyOneByteAtEndOfFile() throws IOException {
+-        try (final SeekableStream stream = new SeekableHTTPStream(new URL(BAM_URL_STRING))){
+-            byte[] buff = new byte[1];
+-            long length = stream.length();
+-            stream.seek(length - 1);
+-            Assert.assertFalse(stream.eof());
+-            Assert.assertEquals(stream.read(buff), 1);
+-            Assert.assertTrue(stream.eof());
+-            Assert.assertEquals(stream.read(buff), -1);
+-        }
+-    }
+-
+-
 -    /**
 -     * Test an attempt to read past the end of the file.  The test file is 594,149 bytes in length.  The test
 -     * attempts to read a 1000 byte block starting at position 594000.  A correct result would return 149 bytes.
@@ -56,7 +70,7 @@ Last-Updated: 2015-01-06
 -        bufferedStream.seek(startPosition);
 -        int bytesRead = bufferedStream.read(buffer, 0, length);
 -        assertEquals(remainder, bytesRead);
- 
+-
 -        // Subsequent reads should return -1
 -        bytesRead = bufferedStream.read(buffer, 0, length);
 -        assertEquals(-1, bytesRead);
@@ -66,7 +80,7 @@ Last-Updated: 2015-01-06
      public void testSkip() throws IOException {
 --- a/src/test/java/htsjdk/samtools/sra/SRAIndexTest.java
 +++ b/src/test/java/htsjdk/samtools/sra/SRAIndexTest.java
-@@ -50,85 +50,4 @@
+@@ -50,85 +50,4 @@ public class SRAIndexTest extends Abstra
      private static final int LAST_BIN_LEVEL = GenomicIndexUtil.LEVEL_STARTS.length - 1;
      private static final int SRA_BIN_OFFSET = GenomicIndexUtil.LEVEL_STARTS[LAST_BIN_LEVEL];
  
@@ -154,7 +168,7 @@ Last-Updated: 2015-01-06
  }
 --- a/src/test/java/htsjdk/samtools/sra/SRATest.java
 +++ b/src/test/java/htsjdk/samtools/sra/SRATest.java
-@@ -56,378 +56,5 @@
+@@ -56,378 +56,5 @@ import java.util.TreeSet;
   */
  public class SRATest extends AbstractSRATest {
  
@@ -535,7 +549,7 @@ Last-Updated: 2015-01-06
  }
 --- a/src/test/java/htsjdk/tribble/util/ftp/FTPClientTest.java
 +++ b/src/test/java/htsjdk/tribble/util/ftp/FTPClientTest.java
-@@ -25,225 +25,4 @@
+@@ -25,225 +25,4 @@ public class FTPClientTest extends Htsjd
      static byte[] expectedBytes = "abcdefghijklmnopqrstuvwxyz\n".getBytes();
      FTPClient client;
  
@@ -763,7 +777,7 @@ Last-Updated: 2015-01-06
  }
 --- a/src/test/java/htsjdk/tribble/readers/TabixReaderTest.java
 +++ b/src/test/java/htsjdk/tribble/readers/TabixReaderTest.java
-@@ -135,30 +135,6 @@
+@@ -135,30 +135,6 @@ public class TabixReaderTest extends Hts
      }
  
      /**
@@ -796,7 +810,7 @@ Last-Updated: 2015-01-06
       * @throws java.io.IOException
 --- a/src/test/java/htsjdk/samtools/cram/ref/EnaRefServiceTest.java
 +++ b/src/test/java/htsjdk/samtools/cram/ref/EnaRefServiceTest.java
-@@ -15,8 +15,4 @@
+@@ -15,8 +15,4 @@ public class EnaRefServiceTest extends H
                  {"0000088cbcebe818eb431d58c908c698"}};
      }
  
@@ -807,7 +821,7 @@ Last-Updated: 2015-01-06
  }
 --- a/src/test/java/htsjdk/samtools/BAMRemoteFileTest.java
 +++ b/src/test/java/htsjdk/samtools/BAMRemoteFileTest.java
-@@ -44,276 +44,7 @@
+@@ -44,276 +44,7 @@ import static org.testng.Assert.*;
  public class BAMRemoteFileTest extends HtsjdkTest {
      private final File BAM_INDEX_FILE = new File("src/test/resources/htsjdk/samtools/BAMFileIndexTest/index_test.bam.bai");
      private final File BAM_FILE = new File("src/test/resources/htsjdk/samtools/BAMFileIndexTest/index_test.bam");
@@ -1086,7 +1100,7 @@ Last-Updated: 2015-01-06
  }
 --- a/src/test/java/htsjdk/tribble/util/ftp/FTPUtilsTest.java
 +++ b/src/test/java/htsjdk/tribble/util/ftp/FTPUtilsTest.java
-@@ -15,18 +15,4 @@
+@@ -15,18 +15,4 @@ import static org.testng.Assert.assertTr
  */
  public class FTPUtilsTest extends HtsjdkTest {
  
@@ -1107,7 +1121,7 @@ Last-Updated: 2015-01-06
  }
 --- a/src/test/java/htsjdk/samtools/seekablestream/SeekableFTPStreamTest.java
 +++ b/src/test/java/htsjdk/samtools/seekablestream/SeekableFTPStreamTest.java
-@@ -40,67 +40,6 @@
+@@ -40,67 +40,6 @@ import java.net.URL;
  public class SeekableFTPStreamTest extends HtsjdkTest {
  
  
@@ -1177,7 +1191,7 @@ Last-Updated: 2015-01-06
  }
 --- a/src/test/java/htsjdk/tribble/util/ParsingUtilsTest.java
 +++ b/src/test/java/htsjdk/tribble/util/ParsingUtilsTest.java
-@@ -153,67 +153,12 @@
+@@ -153,67 +153,12 @@ public class ParsingUtilsTest extends Ht
          tstExists(file.toUri().toString(), false);
      }
  
@@ -1247,9 +1261,9 @@ Last-Updated: 2015-01-06
  }
 --- a/src/test/java/htsjdk/tribble/AbstractFeatureReaderTest.java
 +++ b/src/test/java/htsjdk/tribble/AbstractFeatureReaderTest.java
-@@ -52,54 +52,6 @@
-     //wrapper which skips the first byte of a file and leaves the rest unchanged
+@@ -54,54 +54,6 @@ public class AbstractFeatureReaderTest e
      private static final Function<SeekableByteChannel, SeekableByteChannel> WRAPPER = SkippingByteChannel::new;
+     public static final String REDIRECTING_CODEC_TEST_FILES = "src/test/resources/htsjdk/tribble/AbstractFeatureReaderTest/redirectingCodecTest/";
  
 -    /**
 -     * Asserts readability and correctness of VCF over HTTP.  The VCF is indexed and requires and index.
@@ -1304,7 +1318,7 @@ Last-Updated: 2015-01-06
      private static Object[][] vcfFileAndWrapperCombinations(){
 --- a/src/test/java/htsjdk/variant/PrintVariantsExampleTest.java
 +++ b/src/test/java/htsjdk/variant/PrintVariantsExampleTest.java
-@@ -49,10 +49,12 @@
+@@ -49,10 +49,12 @@ public class PrintVariantsExampleTest ex
                  tempFile.getAbsolutePath()
          };
          Assert.assertEquals(tempFile.length(), 0);

debian/patches/20-fix-version.patch

@@ -30,7 +30,7 @@ Last-Updated: 2016-07-05
  dependencies {
      compile "org.apache.commons:commons-jexl:2.1.1"
      compile "commons-logging:commons-logging:1.1.1"
-@@ -45,9 +33,17 @@ dependencies {
+@@ -46,9 +34,17 @@ dependencies {
  sourceCompatibility = 1.8
  targetCompatibility = 1.8
  

debian/patches/30-disable-scalatest.patch

@@ -15,7 +15,7 @@ Last-Updated: 2017-10-18
      id 'maven'
      id 'signing'
  }
-@@ -23,11 +23,11 @@ dependencies {
+@@ -23,12 +23,12 @@ dependencies {
      compile "org.tukaani:xz:1.5"
      compile "gov.nih.nlm.ncbi:ngs-java:2.9.0"
  
@@ -24,15 +24,17 @@ Last-Updated: 2017-10-18
 -    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 "org.scala-lang:scala-library:2.11.x"
+-    testCompile "com.google.guava:guava:26.0-jre"
++    testCompile "org.scala-lang:scala-library:2.12.x"
 +    //testCompile "org.scalatest:scalatest_2.12:3.0.5"
 +    //testRuntime 'org.pegdown:pegdown:1.6.0' // Necessary for generating HTML reports with ScalaTest
 +    testCompile "org.testng:testng:debian"
 +    testCompile "com.google.jimfs:jimfs:debian"
++    testCompile "com.google.guava:guava:debian"
   }
  
  sourceCompatibility = 1.8
-@@ -90,33 +90,30 @@ test {
+@@ -91,33 +91,30 @@ test {
          jvmArgs += '-Dsamjdk.sra_libraries_download=true'
      }
  

debian/patches/40-fix-java9

@@ -6,7 +6,7 @@ Last-Updated: 17-04-2018
 Forwarded: no
 --- a/build.gradle
 +++ b/build.gradle
-@@ -124,6 +124,7 @@ wrapper {
+@@ -125,6 +125,7 @@ wrapper {
  if (JavaVersion.current().isJava8Compatible()) {
      tasks.withType(Javadoc) {
          options.addStringOption('Xdoclint:none', '-quiet')

debian/patches/60-enable-tests

@@ -4,7 +4,7 @@ Description: Enable tests using testng
 
 --- a/build.gradle
 +++ b/build.gradle
-@@ -86,9 +86,14 @@ test {
+@@ -87,9 +87,14 @@ test {
          events "failed", "skipped"
      }
  

debian/patches/90-disable-testftp.patch

@@ -4,7 +4,7 @@ Description: remove unsupported *tags* and disable ftp tests as require network
 
 --- a/build.gradle
 +++ b/build.gradle
-@@ -102,15 +102,6 @@ test {
+@@ -103,15 +103,6 @@ test {
  } dependsOn findScalaAndJavaTypes
  
  

src/main/java/htsjdk/samtools/AbstractSAMHeaderRecord.java

@@ -23,7 +23,7 @@
  */
 package htsjdk.samtools;
 
-import javax.xml.bind.annotation.XmlTransient;
+
 import java.io.Serializable;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -33,7 +33,7 @@ import java.util.Set;
  * Base class for the various concrete records in a SAM header, providing uniform
  * access to the attributes.
  */
-@XmlTransient /* don't consider this class for XML-serialization */
+
 public abstract class AbstractSAMHeaderRecord implements Serializable {
     public static final long serialVersionUID = 1L;
 

src/main/java/htsjdk/samtools/BAMRecord.java

@@ -310,7 +310,7 @@ public class BAMRecord extends SAMRecord {
      * extracts the CIGAR from the CG tag and places it into the (in memory) cigar.
      */
     private void extractCigarFromCGAttribute(final Cigar sentinelCigar) throws IllegalStateException {
-        final int[] cigarFromCG = (int[]) getAttribute(SAMTagUtil.getSingleton().CG);
+        final int[] cigarFromCG = (int[]) getAttribute(SAMTag.CG.getBinaryTag());
 
         if (cigarFromCG == null) return;
 
@@ -353,7 +353,7 @@ public class BAMRecord extends SAMRecord {
         initializeCigar(decodedCigar);
 
         // remove CG attribute.
-        setAttribute(SAMTagUtil.getSingleton().CG, null);
+        setAttribute(SAMTag.CG.getBinaryTag(), null);
     }
 
     /**

src/main/java/htsjdk/samtools/CRAMIterator.java

@@ -34,10 +34,7 @@ import htsjdk.samtools.util.Log;
 import java.io.IOException;
 import java.io.InputStream;
 import java.math.BigInteger;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
 
 import htsjdk.samtools.cram.CRAMException;
 
@@ -259,9 +256,7 @@ public class CRAMIterator implements SAMRecordIterator {
         if (!iterator.hasNext()) {
             try {
                 nextContainer();
-            } catch (IOException e) {
-                throw new SAMException(e);
-            } catch (IllegalAccessException e) {
+            } catch (IOException | IllegalAccessException e) {
                 throw new SAMException(e);
             }
         }
@@ -271,7 +266,11 @@ public class CRAMIterator implements SAMRecordIterator {
 
     @Override
     public SAMRecord next() {
+        if (hasNext()) {
             return iterator.next();
+        } else {
+            throw new NoSuchElementException();
+        }
     }
 
     @Override

src/main/java/htsjdk/samtools/FixBAMFile.java

-/*
- * The MIT License
- *
- * Copyright (c) 2010 The Broad Institute
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- */
-package htsjdk.samtools;
-
-
-import htsjdk.samtools.util.CloserUtil;
-
-import java.io.File;
-
-/**
- * @deprecated since 07/2017. This tool is undocumented and untested.
- */
-@Deprecated
-public class FixBAMFile {
-    public static void main(String[] args) {
-        File inputFile = new File(args[0]);
-        File outputFile = new File(args[1]);
-        SamReader reader = SamReaderFactory.makeDefault().validationStringency(ValidationStringency.SILENT).open(inputFile);
-        SAMFileHeader header = reader.getFileHeader();
-        SAMFileWriter writer = new SAMFileWriterFactory().makeBAMWriter(header, true, outputFile);
-        for (SAMRecord record : reader) {
-            if (record.getIndexingBin() != null) {
-                record.setIndexingBin(record.computeIndexingBin());
-            }
-            writer.addAlignment(record);
-        }
-        writer.close();
-        CloserUtil.close(reader);
-    }
-}

src/main/java/htsjdk/samtools/LinearIndex.java

@@ -24,6 +24,7 @@
 package htsjdk.samtools;
 
 import java.util.Arrays;
+import java.util.Objects;
 
 /**
  * The linear index associated with a given reference in a BAM index.
@@ -103,23 +104,19 @@ public class LinearIndex {
     }
 
     @Override
-    public boolean equals(final Object o) {
+    public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
-
-        final LinearIndex that = (LinearIndex) o;
-
-        if (mIndexStart != that.mIndexStart) return false;
-        if (mReferenceSequence != that.mReferenceSequence) return false;
-        if (!Arrays.equals(mIndexEntries, that.mIndexEntries)) return false;
-
-        return true;
+        LinearIndex that = (LinearIndex) o;
+        return mReferenceSequence == that.mReferenceSequence &&
+                mIndexStart == that.mIndexStart &&
+                Arrays.equals(mIndexEntries, that.mIndexEntries);
     }
 
     @Override
     public int hashCode() {
-        int result = mReferenceSequence;
-        result = 31 * result + mIndexStart;
+
+        int result = Objects.hash(mReferenceSequence, mIndexStart);
         result = 31 * result + Arrays.hashCode(mIndexEntries);
         return result;
     }

src/main/java/htsjdk/samtools/SAMBinaryTagAndUnsignedArrayValue.java

@@ -35,7 +35,7 @@ public class SAMBinaryTagAndUnsignedArrayValue extends SAMBinaryTagAndValue {
         if (!value.getClass().isArray() || value instanceof float[]) {
             throw new IllegalArgumentException("Attribute type " + value.getClass() +
                     " cannot be encoded as an unsigned array. Tag: " +
-                    SAMTagUtil.getSingleton().makeStringTag(tag));
+                    SAMTag.makeStringTag(tag));
         }
     }
 

src/main/java/htsjdk/samtools/SAMBinaryTagAndValue.java

@@ -59,7 +59,7 @@ public class SAMBinaryTagAndValue implements Serializable {
         }
         if (!isAllowedAttributeValue(value)) {
             throw new IllegalArgumentException("Attribute type " + value.getClass() + " not supported. Tag: " +
-                    SAMTagUtil.getSingleton().makeStringTag(tag));
+                    SAMTag.makeStringTag(tag));
         }
         this.tag = tag;
         this.value = value;

src/main/java/htsjdk/samtools/SAMFileHeader.java

@@ -29,16 +29,8 @@ import htsjdk.samtools.util.CollectionUtil;
 import htsjdk.samtools.util.Log;
 
 import java.io.StringWriter;
-import java.lang.reflect.Constructor;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
+import java.util.function.Supplier;
 
 /**
  * Header information from a SAM or BAM file.
@@ -48,8 +40,8 @@ public class SAMFileHeader extends AbstractSAMHeaderRecord
     public static final String VERSION_TAG = "VN";
     public static final String SORT_ORDER_TAG = "SO";
     public static final String GROUP_ORDER_TAG = "GO";
-    public static final String CURRENT_VERSION = "1.5";
-    public static final Set<String> ACCEPTABLE_VERSIONS = CollectionUtil.makeSet("1.0", "1.3", "1.4", "1.5");
+    public static final String CURRENT_VERSION = "1.6";
+    public static final Set<String> ACCEPTABLE_VERSIONS = CollectionUtil.makeSet("1.0", "1.3", "1.4", "1.5", CURRENT_VERSION );
 
     private SortOrder sortOrder = null;
     private GroupOrder groupOrder = null;
@@ -70,40 +62,32 @@ public class SAMFileHeader extends AbstractSAMHeaderRecord
      * Ways in which a SAM or BAM may be sorted.
      */
     public enum SortOrder {
-        unsorted(null),
-        queryname(SAMRecordQueryNameComparator.class),
-        coordinate(SAMRecordCoordinateComparator.class),
-        duplicate(SAMRecordDuplicateComparator.class), // NB: this is not in the SAM spec!
-        unknown(null);
+        unsorted(() -> null),
+        queryname(SAMRecordQueryNameComparator::new),
+        coordinate(SAMRecordCoordinateComparator::new),
+        duplicate(SAMRecordDuplicateComparator::new), // NB: this is not in the SAM spec!
+        unknown(() -> null);
 
-        private final Class<? extends SAMRecordComparator> comparator;
+        private final Supplier<SAMRecordComparator> comparatorSupplier;
 
-        SortOrder(final Class<? extends SAMRecordComparator> comparatorClass) {
-            this.comparator = comparatorClass;
+        SortOrder(final Supplier<SAMRecordComparator> comparatorClass) {
+            this.comparatorSupplier = comparatorClass;
         }
 
         /**
          * @return Comparator class to sort in the specified order, or null if unsorted.
+         * @deprecated since 7/2018, use {@link #getComparatorInstance()} with {@link SAMSortOrderChecker#getClass()} instead
          */
+        @Deprecated
         public Class<? extends SAMRecordComparator> getComparator() {
-            return comparator;
+            return comparatorSupplier.get().getClass();
         }
 
         /**
          * @return Comparator to sort in the specified order, or null if unsorted.
          */
         public SAMRecordComparator getComparatorInstance() {
-            if (comparator != null) {
-                try {
-                    final Constructor<? extends SAMRecordComparator> ctor = comparator.getConstructor();
-                    return ctor.newInstance();
-                }
-                catch (Exception e) {
-                    throw new IllegalStateException("Could not instantiate a comparator for sort order: " +
-                            this.name(), e);
-                }
-            }
-            return null;
+            return comparatorSupplier.get();
         }
     }
 
@@ -262,7 +246,7 @@ public class SAMFileHeader extends AbstractSAMHeaderRecord
                 try {
                     return SortOrder.valueOf(so);
                 } catch (IllegalArgumentException e) {
-                    log.warn("Found non conforming header SO tag: " + so + ". Treating as 'unknown'.");
+                    log.warn("Found non-conforming header SO tag: " + so + ". Treating as 'unknown'.");
                     sortOrder = SortOrder.unknown;
                 }
             }
@@ -324,12 +308,18 @@ public class SAMFileHeader extends AbstractSAMHeaderRecord
      */
     @Override
     public void setAttribute(final String key, final String value) {
+        String tempVal = value;
         if (key.equals(SORT_ORDER_TAG)) {
             this.sortOrder = null;
+            try {
+                tempVal = SortOrder.valueOf(value).toString();
+            } catch (IllegalArgumentException e) {
+                tempVal = SortOrder.unknown.toString();
+            }
         } else if (key.equals(GROUP_ORDER_TAG)) {
             this.groupOrder = null;
         }
-        super.setAttribute(key, value);
+        super.setAttribute(key, tempVal);
     }
 
     /**

src/main/java/htsjdk/samtools/SAMFileWriterImpl.java

@@ -156,7 +156,7 @@ public abstract class SAMFileWriterImpl implements SAMFileWriter
             }
         } else if (!sortOrder.equals(SAMFileHeader.SortOrder.unsorted)) {
             alignmentSorter = SortingCollection.newInstance(SAMRecord.class,
-                    new BAMRecordCodec(header), makeComparator(), maxRecordsInRam, tmpDir);
+                    new BAMRecordCodec(header), sortOrder.getComparatorInstance(), maxRecordsInRam, tmpDir);
         }
     }
 
@@ -165,20 +165,6 @@ public abstract class SAMFileWriterImpl implements SAMFileWriter
         return header;
     }
 
-    private SAMRecordComparator makeComparator() {
-        switch (sortOrder) {
-            case coordinate:
-                return new SAMRecordCoordinateComparator();
-            case queryname:
-                return new SAMRecordQueryNameComparator();
-            case duplicate:
-                return new SAMRecordDuplicateComparator();
-            case unsorted:
-                return null;
-        }
-        throw new IllegalStateException("sortOrder should not be null");
-    }
-
     /**
      * Add an alignment record to be emitted by the writer.
      *

src/main/java/htsjdk/samtools/SAMLineParser.java

@@ -465,10 +465,9 @@ public class SAMLineParser {
     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);
         }
     }