Changes to address yfarjoun's review comments.

src/main/java/picard/analysis/directed/TargetMetricsCollector.java

@@ -430,7 +430,7 @@ public Map<Interval, Coverage> getCoverageByTarget() {
  /** Adds information about an individual SAMRecord to the statistics. */
  public void acceptRecord(final SAMRecord record) {
  // Just ignore secondary alignments altogether
- if (record.getNotPrimaryAlignmentFlag()) return;
+ if (record.isSecondaryAlignment()) return;
 
  // Cache some things, and compute the total number of bases aligned in the record.
  final boolean mappedInPair = record.getReadPairedFlag() && !record.getReadUnmappedFlag() && !record.getMateUnmappedFlag() && !record.getSupplementaryAlignmentFlag();
@@ -583,10 +583,10 @@ public void acceptRecord(final SAMRecord record) {
  }
 
  // Then go through the per-target/per-base hq and unfiltered coverage
- // The cutoff of >= 2 is because even the unfilteredCoverage doesn't want Q1 or Q0 bases!
+ // The cutoff of > 2 is because even the unfilteredCoverage doesn't want those bases
  if (qual > 2 && incrementPerTargetCoverage && onTarget) {
  for (final Interval target : targets) {
- if (refPos >= target.getStart() && refPos <= target.getEnd()) {
+ if (overlapsInterval(refPos, target)) {
  final int targetOffset = refPos - target.getStart();
 
  // Unfiltered first (for theoretical het sensitivity)
@@ -618,11 +618,16 @@ public void acceptRecord(final SAMRecord record) {
  /* Returns true if the `pos` is between the `start` and `end` of at least one interval. */
  private boolean overlapsAny(final int pos, final Collection<Interval> intervals) {
  for (final Interval interval : intervals) {
- if (pos >= interval.getStart() && pos <= interval.getEnd()) return true;
+ if (overlapsInterval(pos, interval)) return true;
  }
  return false;
  }
 
+ /** Returns true if the position is within the start-end range inclusive of the given interval. */
+ private boolean overlapsInterval(final int pos, final Interval interval) {
+ return pos >= interval.getStart() && pos <= interval.getEnd();
+ }
+
  @Override
  public void finish() {
  metrics.PCT_PF_READS = metrics.PF_READS / (double) metrics.TOTAL_READS;

src/test/java/picard/analysis/directed/CollectHsMetricsTest.java

@@ -61,9 +61,9 @@ HsMetrics readMetrics(final File f) {
 
  /** Writes the contents of a SAMRecordSetBuilder out to a file. */
  File writeBam(final SAMRecordSetBuilder builder, final File f) {
- final SAMFileWriter out = new SAMFileWriterFactory().makeSAMOrBAMWriter(builder.getHeader(), false, f);
- builder.forEach(out::addAlignment);
- out.close();
+ try (final SAMFileWriter out = new SAMFileWriterFactory().makeSAMOrBAMWriter(builder.getHeader(), false, f)) {
+  builder.forEach(out::addAlignment);
+ }
  return f;
  }
 
@@ -187,6 +187,8 @@ public void testHsMetricsHandlesIndelsAppropriately() throws IOException {
  runPicardCommandLine(Arrays.asList("INCLUDE_INDELS=true", "SAMPLE_SIZE=0", "TI="+ts.getPath(), "BI="+bs.getPath(), "O="+out.getPath(), "I="+withInsBam.getAbsolutePath()));
  final HsMetrics insWithIndelHandling = readMetrics(out);
 
+ IOUtil.deleteDirectoryTree(dir);
+
  Assert.assertEquals(delsWithoutIndelHandling.MEAN_TARGET_COVERAGE, 90.0); // 100X over 180/200 bases due to deletion
  Assert.assertEquals(delsWithIndelHandling.MEAN_TARGET_COVERAGE, 100.0); // 100X with counting the deletion