FLOW_* parameters must be accompanied by FLOW_MODE

broadinstitute · Oct 13, 2024 · 233cf9d · 233cf9d
1 parent 095b989
commit 233cf9d
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 7 deletions.
diff --git a/src/main/java/picard/sam/markduplicates/MarkDuplicates.java b/src/main/java/picard/sam/markduplicates/MarkDuplicates.java
@@ -27,9 +27,7 @@
 import htsjdk.samtools.*;
 import htsjdk.samtools.DuplicateScoringStrategy.ScoringStrategy;
 import htsjdk.samtools.util.*;
-import org.broadinstitute.barclay.argparser.Argument;
-import org.broadinstitute.barclay.argparser.ArgumentCollection;
-import org.broadinstitute.barclay.argparser.CommandLineProgramProperties;
+import org.broadinstitute.barclay.argparser.*;
 import org.broadinstitute.barclay.help.DocumentedFeature;
 import picard.PicardException;
 import picard.cmdline.programgroups.ReadDataManipulationProgramGroup;
@@ -263,6 +261,8 @@ protected int doWork() {
         // use flow based calculation helper?
         if ( flowBasedArguments.FLOW_MODE ) {
             calcHelper = new MarkDuplicatesForFlowHelper(this);
+        } else {
+            ensureNoFlowParametersSpecified();
         }
 
         reportMemoryStats("Start of doWork");
@@ -450,6 +450,19 @@ protected int doWork() {
         return 0;
     }
 
+    // make sure no FLOW__ parameter was set (except for FLOW_MODE)
+    private void ensureNoFlowParametersSpecified() {
+        if ( getCommandLineParser() instanceof CommandLineArgumentParser ) {
+            final CommandLineArgumentParser parser = (CommandLineArgumentParser)getCommandLineParser();
+            for (NamedArgumentDefinition def : parser.getNamedArgumentDefinitions()) {
+                final String name = def.getLongName();
+                if (name != "FLOW_MODE" && name.startsWith("FLOW_") && def.getHasBeenSet()) {
+                    throw new PicardException(name + " specified, but no FLOW_MODE");
+                }
+            }
+        }
+    }
+
     /**
      * package-visible for testing
      */

diff --git a/src/test/java/picard/sam/markduplicates/MarkDuplicatesForFlowHelperTest.java b/src/test/java/picard/sam/markduplicates/MarkDuplicatesForFlowHelperTest.java
@@ -299,7 +299,16 @@ public void modify(final AbstractMarkDuplicatesCommandLineProgramTester tester)
     }
 
     @Test(dataProvider = "forFlowDataProvider")
-    public void testForFlowMDCall(final DuplicateScoringStrategy.ScoringStrategy scoringStrategy, final TestRecordInfo[] recInfos, final String[] params, TesterModifier modifier) {
+    public void testForFlowMDCall_WithFlowMode(final DuplicateScoringStrategy.ScoringStrategy scoringStrategy, final TestRecordInfo[] recInfos, final String[] params, TesterModifier modifier) {
+        testForFlowMDCall(scoringStrategy, recInfos, params, modifier, true);
+    }
+
+    @Test(dataProvider = "forFlowDataProvider")
+    public void testForFlowMDCall_WithoutFlowMode(final DuplicateScoringStrategy.ScoringStrategy scoringStrategy, final TestRecordInfo[] recInfos, final String[] params, TesterModifier modifier) {
+        testForFlowMDCall(scoringStrategy, recInfos, params, modifier, false);
+    }
+
+    private void testForFlowMDCall(final DuplicateScoringStrategy.ScoringStrategy scoringStrategy, final TestRecordInfo[] recInfos, final String[] params, TesterModifier modifier, final boolean withFlowMode) {
 
         // get tester, build records
         final AbstractMarkDuplicatesCommandLineProgramTester tester =
@@ -328,7 +337,9 @@ public void testForFlowMDCall(final DuplicateScoringStrategy.ScoringStrategy sco
         }
 
         // add parames, set flow order
-        tester.addArg("FLOW_MODE=true");
+        if ( withFlowMode ) {
+            tester.addArg("FLOW_MODE=true");
+        }
         for ( final String param : params ) {
             tester.addArg(param);
         }
@@ -339,8 +350,12 @@ public void testForFlowMDCall(final DuplicateScoringStrategy.ScoringStrategy sco
             modifier.modify(tester);
         }
 
-        // run test
-        tester.runTest();
+        // run test. tests without flow mode should fail
+        if ( withFlowMode ) {
+            tester.runTest();
+        } else {
+            Assert.assertThrows(() -> tester.runTest());
+        }
     }
 
     @DataProvider(name ="getFlowSumOfBaseQualitiesDataProvider")