organize files

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

@@ -62,7 +62,6 @@
 import picard.nio.PicardHtsPath;
 import picard.util.TabbedTextFileWithHeaderParser;
 
-import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -250,7 +249,6 @@ public String getHelpDoc() {
     protected String[] customCommandLineValidation() {
         final List<String> errors = new ArrayList<>();
         ValidationUtil.validateSanitizeSortOrder(SANITIZE, SORT_ORDER, errors);
-        // tsato: can output or output map be null, and can I streamline this code?
         ValidationUtil.validateOutputParams(OUTPUT_BY_READGROUP, OUTPUT, OUTPUT_MAP, errors);
 
         if (!SANITIZE && KEEP_FIRST_DUPLICATE) errors.add("KEEP_FIRST_DUPLICATE cannot be used without SANITIZE");
@@ -263,7 +261,9 @@ protected String[] customCommandLineValidation() {
 
     protected int doWork() {
         IOUtil.assertFileIsReadable(INPUT.toPath());
-        // ValidationUtil.assertWritable(OUTPUT, OUTPUT_BY_READGROUP); // tsato: figure this out later
+        if (OUTPUT != null && !PicardBucketUtils.isGcsUrl(OUTPUT)) {
+            ValidationUtil.assertWritable(OUTPUT.toPath(), OUTPUT_BY_READGROUP);
+        }
 
         final boolean sanitizing = SANITIZE;
         final SamReader in = SamReaderFactory.makeDefault().referenceSequence(referenceSequence.getReferencePath()).validationStringency(VALIDATION_STRINGENCY).open(INPUT.toPath());
@@ -579,7 +579,7 @@ static Map<String, Path> createOutputMap(
 
         final Map<String, Path> outputMap;
         if (outputMapFile != null) {
-            outputMap = createOutputMapFromFile(outputMapFile); // tsato: need to investigate this path too
+            outputMap = createOutputMapFromFile(outputMapFile);
         } else {
             outputMap = createOutputMap(readGroups, outputDir, defaultExtension);
         }
@@ -605,7 +605,6 @@ private static Map<String, Path> createOutputMapFromFile(final Path outputMapFil
 
     }
 
-    // tsato: some of this can be IOPath or PicardHtsPath
     private static Map<String, Path> createOutputMap(final List<SAMReadGroupRecord> readGroups, final Path outputDir, final String extension) {
         final Map<String, Path> outputMap = new HashMap<>();
         for (final SAMReadGroupRecord readGroup : readGroups) {
@@ -800,7 +799,7 @@ List<PeekableIterator<SAMRecord>> iterators() {
     /**
      * Methods used for validating parameters to RevertSam.
      */
-    static class ValidationUtil {
+    static class ValidationUtil { // tsato: I vote that we rename this to avoid mixing it up with htsjdk ValidationUtils
 
         static void validateSanitizeSortOrder(final boolean sanitize, final SAMFileHeader.SortOrder sortOrder, final List<String> errors) {
             if (sanitize && sortOrder != SAMFileHeader.SortOrder.queryname) {
@@ -894,7 +893,7 @@ static void validateHeaderOverrides(
             }
         }
 
-        static void assertWritable(final File output, final boolean outputByReadGroup) {
+        static void assertWritable(final Path output, final boolean outputByReadGroup) {
             if (outputByReadGroup) {
                 if (output != null) {
                     IOUtil.assertDirectoryIsWritable(output);

src/test/java/picard/sam/RevertSamTest.java

@@ -59,15 +59,15 @@
  * To change this template use File | Settings | File Templates.
  */
 public class RevertSamTest extends CommandLineProgramTest {
-    private static final String basicSamToRevert = "testdata/picard/sam/revert_sam_basic.sam"; // tsato: move these to MergeSam subdirectorty
-    private static final String sampleLibraryOverrideSam = "testdata/picard/sam/revert_sam_sample_library_override.sam";
-    private static final File validOutputMap = new File("testdata/picard/sam/revert_sam_valid_output_map.txt");
-    private static final File nonExistentOutputMap = new File("testdata/picard/sam/revert_sam_does_not_exist.txt");
-    private static final File badHeaderOutputMap = new File("testdata/picard/sam/revert_sam_bad_header_output_map.txt");
+    private static final String basicSamToRevert = "testdata/picard/sam/RevertSam/revert_sam_basic.sam";
+    private static final String sampleLibraryOverrideSam = "testdata/picard/sam/RevertSam/revert_sam_sample_library_override.sam";
+    private static final File validOutputMap = new File("testdata/picard/sam/RevertSam/revert_sam_valid_output_map.txt");
+    private static final File nonExistentOutputMap = new File("testdata/picard/sam/RevertSam/revert_sam_does_not_exist.txt");
+    private static final File badHeaderOutputMap = new File("testdata/picard/sam/RevertSam/revert_sam_bad_header_output_map.txt");
     private static final File samTestData = new File("testdata/picard/sam");
-    private static final File writablePath = new File("testdata/picard/sam/revert_sam_writable.bam");
+    private static final File writablePath = new File("testdata/picard/sam/RevertSam/revert_sam_writable.bam");
     private static final File referenceFasta = new File("testdata/picard/reference/test.fasta");
-    private static final String singleEndSamToRevert = "testdata/picard/sam/revert_sam_single_end.sam";
+    private static final String singleEndSamToRevert = "testdata/picard/sam/RevertSam/revert_sam_single_end.sam";
     private static final File hardClipFasta = new File("testdata/picard/sam/MergeBamAlignment/cliptest.fasta");
     private static final File hardClippedAlignedSam = new File("testdata/picard/sam/MergeBamAlignment/hardclip.aligned.sam");
     private static final File hardClippedUnmappedSam = new File("testdata/picard/sam/MergeBamAlignment/hardclip.unmapped.sam");
@@ -480,12 +480,12 @@ public void testFilePathsWithoutMapFile() {
         final SAMReadGroupRecord rg2 = new SAMReadGroupRecord("rg2");
 
         final Map<String, Path> outputMap = RevertSam.createOutputMap(null, new File("/foo/bar").toPath(), ".bam", Arrays.asList(rg1, rg2));
-        Assert.assertEquals(outputMap.get("rg1"), new File("/foo/bar/rg1.bam").toPath()); // tsato: questionable wrapping but ok for now
+        Assert.assertEquals(outputMap.get("rg1"), new File("/foo/bar/rg1.bam").toPath());
         Assert.assertEquals(outputMap.get("rg2"), new File("/foo/bar/rg2.bam").toPath());
     }
 
     @Test
-    public void testFilePathsWithMapFile() { // tsato: perhaps best to use PicardHtsPath instead of putting File.toPath everywhere
+    public void testFilePathsWithMapFile() {
         final Map<String, Path> outputMap = RevertSam.createOutputMap(validOutputMap.toPath(), null, ".bam", Collections.emptyList());
         Assert.assertEquals(outputMap.get("rg1"), new File("/path/to/my_rg_1.ubam").toPath());
         Assert.assertEquals(outputMap.get("rg2"), new File("/path/to/my_rg_2.ubam").toPath());