Extract tests from PrintReadsIntegrationTest to share with Spark version #5689
Conversation
|
|
||
| @Test( groups = "spark") | ||
| @Test() | ||
| public void testUnSortedRoundTrip() throws Exception { |
There was a problem hiding this comment.
This is the only test that is Spark-specific. It fails on PrintReads (non-Spark) since that tool does not allow reads to be written in an order that is inconsistent with the header (the header says coordinate sorted, but the file is actually queryname sorted).
Also (contrary to the comment in this test) PrintReadSpark does not sort the file, so it writes an invalid BAM. This seems to be wrong.
There was a problem hiding this comment.
Right, so the point with this test as I added it was to enforce that printReadsSpark was no longer spuriously sorting the output reads (at runtime cost and some pain if you were dealing with invalid input files in the first place). I'm not sure its the place of PrintReads to go about sorting your bam output for you, especially if your purpose is to extract reads with some tag or another. Perhaps this test would make more sense if we were looking at queryname sorted bams, where there exists a valid alternative version of what a "queryname" sort means that somebody might well want to preserve.
Codecov Report
@@ Coverage Diff @@
## master #5689 +/- ##
===============================================
- Coverage 87.069% 87.064% -0.005%
+ Complexity 31875 31850 -25
===============================================
Files 1940 1941 +1
Lines 146738 146599 -139
Branches 16226 16208 -18
===============================================
- Hits 127764 127635 -129
+ Misses 13061 13057 -4
+ Partials 5913 5907 -6
|
|
|
||
| @Test( groups = "spark") | ||
| @Test() | ||
| public void testUnSortedRoundTrip() throws Exception { |
There was a problem hiding this comment.
Right, so the point with this test as I added it was to enforce that printReadsSpark was no longer spuriously sorting the output reads (at runtime cost and some pain if you were dealing with invalid input files in the first place). I'm not sure its the place of PrintReads to go about sorting your bam output for you, especially if your purpose is to extract reads with some tag or another. Perhaps this test would make more sense if we were looking at queryname sorted bams, where there exists a valid alternative version of what a "queryname" sort means that somebody might well want to preserve.
|
|
||
| SamAssertionUtils.assertSamsEqual(outBam, inBam); | ||
| } | ||
| public final class PrintReadsSparkIntegrationTest extends AbstractPrintReadsIntegrationTest { |
There was a problem hiding this comment.
We still want these tests to be tagged as spark tests I think, i'm not sure how the propagation for testng works with inherited tests but these tests should probably stay in the spark group. If you added @Test(groups = {"spark"}) to the class I suspect this might propagate the test tags correctly.
There was a problem hiding this comment.
You can check by looking at the travis test outputs to check that these did indeed run with the spark tests.
There was a problem hiding this comment.
I added the spark group back. Also, these tests were all run as a part of the integration test type.
tomwhite
force-pushed
the
tw_common_print_reads_integration_tests
branch
from
4b48fe9
to
200ddaf
Compare
PrintReadsIntegrationTest and PrintReadsSparkIntegrationTest had diverged, this fixes the problem.