Louis

src/main/java/picard/sam/RevertSam.java

@@ -269,6 +269,12 @@ protected int doWork() {
         }
 
         final boolean sanitizing = SANITIZE;
+
+        // Wrap the INPUT.toPath() inside SamInputResource.of() as a workaround for the Illegal Seek error.
+        // For details, see:
+        //   https://github.com/broadinstitute/picard/pull/1974
+        //   https://github.com/samtools/htsjdk/pull/1124
+        //   https://github.com/samtools/htsjdk/issues/1084
         final SamReader in = SamReaderFactory.makeDefault().referenceSequence(referenceSequence.getReferencePath()).validationStringency(VALIDATION_STRINGENCY).open(SamInputResource.of(INPUT.toPath()));
         final SAMFileHeader inHeader = in.getFileHeader();
         ValidationUtil.validateHeaderOverrides(inHeader, SAMPLE_ALIAS, LIBRARY_NAME);

src/test/java/picard/sam/PipedDataTest.java

@@ -1,18 +1,22 @@
 package picard.sam;
 
+import htsjdk.samtools.SamReader;
+import htsjdk.samtools.SamReaderFactory;
 import org.testng.Assert;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.stream.StreamSupport;
 
 import static picard.sam.MergeBamAlignmentTest.fasta;
 import static picard.sam.MergeBamAlignmentTest.unmappedBam;
 
 public class PipedDataTest {
-    private final String classPath = "\"" + System.getProperty("java.class.path") + "\" ";
-    private final String picardCommandlinePreamble = "java -classpath " + classPath + "picard.cmdline.PicardCommandLine ";
+    private static final String classPath = "\"" + System.getProperty("java.class.path") + "\" ";
+    private static final String picardCommandlinePreamble = "java -classpath " + classPath + "picard.cmdline.PicardCommandLine ";
+    private static final String TEST_BAM = "testdata/picard/sam/test.bam";
 
     /**
      * Creates the command line argument to be used for piped (/dev/stdin) input tests.
@@ -32,63 +36,74 @@ private String getViewSamPicardCommand(final String inputSAM){
     public Object[][] getPipedInputTestData() throws IOException {
         // Note the trailing space in each string block.
         // TODO: port ArgumentBuilder from GATK so this would not be necessary.
+        final File sortSamOutput = File.createTempFile("SortSam_pipe_test", "bam");
+        sortSamOutput.deleteOnExit();
         final String[] sortSamCommand = {
                 "/bin/bash",
                 "-c",
-                getViewSamPicardCommand("testdata/picard/sam/test.bam") +
+                getViewSamPicardCommand(TEST_BAM) +
                 "| " +
                 picardCommandlinePreamble +
                 "SortSam " +
+                "SORT_ORDER=queryname " +
                 "I=/dev/stdin " +
-                "O=/dev/null " +
-                "SORT_ORDER=queryname"
+                "O=" + sortSamOutput.getAbsolutePath()
         };
 
-        final File temp = File.createTempFile("test", "sam");
-        temp.deleteOnExit();
+        final File revertSamOutput = File.createTempFile("RevertSam_pipe_test", "bam");
+        revertSamOutput.deleteOnExit();
         final String[] revertSamCommand = {
             "/bin/bash",
             "-c",
-            getViewSamPicardCommand("testdata/picard/sam/test.bam") +
+            getViewSamPicardCommand(TEST_BAM) +
             "| " +
             picardCommandlinePreamble +
             "RevertSam " +
             "I=/dev/stdin " +
-            "O=/dev/null "
+            "O=" + revertSamOutput.getAbsolutePath()
         };
 
-        // Only test the case where the "alignedBam" argument is /dev/stdin, which is the standard use case (bwa -> mergebamalignment)
-        // Make sure that the input aligned bam is query-name sorted (as it would be if it's piped from bwa)
+
+        // Only test the case where the "alignedBam" argument is /dev/stdin, which is the standard use case (bwa -> MergeBamAlignment).
+        //
+        // Note that the input aligned bam must be query-name sorted (as it would be if it's piped from bwa).
         // MergeBamAlignment fails if the coordinate sorted aligned bam is given, after trying to query-name sort dynamically and failing to find the reference header items,
-        // due seemingly to the fact that the input is /dev/stdin.
+        // seemingly due to the fact that the input is /dev/stdin.
+        final File mergeBamAlignmentOutput = File.createTempFile("MergeBamAlignment_pipe_test", "bam");
+        mergeBamAlignmentOutput.deleteOnExit();
         final String[] mergeBamAlignmentCommand = {
                 "/bin/bash",
                 "-c",
-                getViewSamPicardCommand(MergeBamAlignmentTest.alignedQuerynameSortedBam.getAbsolutePath()) + // here we have to use "+", not ",". Best to extract a method "piped input argument builder" or something
+                getViewSamPicardCommand(MergeBamAlignmentTest.alignedQuerynameSortedBam.getAbsolutePath()) + // here we have to use "+", not ",".
                 "| " +
                 picardCommandlinePreamble +
                 "MergeBamAlignment " +
                 "ALIGNED_BAM=/dev/stdin " +
                 "UNMAPPED=" + unmappedBam + " " +
                 "REFERENCE_SEQUENCE=" + fasta + " " +
-                "OUTPUT=/dev/null "
+                "OUTPUT=" + mergeBamAlignmentOutput.getAbsolutePath()
         };
 
         return new Object[][]{
-                {sortSamCommand},
-                {revertSamCommand},
-                {mergeBamAlignmentCommand}
+                {sortSamCommand, sortSamOutput},
+                {revertSamCommand, revertSamOutput},
+                {mergeBamAlignmentCommand, mergeBamAlignmentOutput}
         };
     }
 
     @Test(dataProvider = "pipedInputTestData")
-    public void testPipedInput(final String[] command) {
+    public void testPipedInput(final String[] command, final File tmpOutput) {
         try {
             ProcessBuilder processBuilder = new ProcessBuilder(command);
             processBuilder.inheritIO();
 
             Process process = processBuilder.start();
             Assert.assertEquals(process.waitFor(), 0);
+
+            final SamReader reader = SamReaderFactory.makeDefault().open(tmpOutput);
+            final long numOutputReads = StreamSupport.stream(reader.spliterator(), false).count();
+            Assert.assertTrue(numOutputReads > 0);
+            Assert.assertTrue(reader.getFileHeader().getReadGroups().size() > 0);
         } catch (Exception e) {
             Assert.fail("Failed to pipe data to picard. The error was: ", e);
         }