IndexFeatureFile: more informative error message when trying to index a malformed file #4187
Conversation
|
@ldgauthier please review |
| @@ -169,7 +169,7 @@ public void testVCFGZIndex_inferredName(){ | |||
| checkIndex(index, Arrays.asList("1", "2", "3", "4")); | |||
| } | |||
|
|
|||
| @Test(expectedExceptions = UserException.MalformedFile.class) | |||
There was a problem hiding this comment.
@lbergelson and I have lamented how many of our tests expect non-subclassed UserExceptions and then get called with missing input files and everything passes. I'd be happier if IndexFeatureFile threw a more specific UserException, but I understand you're lumping together a few different cases. Maybe add a UserException.TribbleParseError or something similarly named?
There was a problem hiding this comment.
Unfortunately the underlying cause here can be anything from "file is malformed" to "codec does not support tabix", and it all comes out of htsjdk as a TribbleException. I thought about using UserException.MalformedFile here, which will be accurate 90%+ of the time, but there is that other 10%...
There was a problem hiding this comment.
Over time every test that catches raw UserException breaks and starts to pass incorrectly due to changes in the command line inputs.
There was a problem hiding this comment.
Yeah, it's hard when all we know is "Something is wrong"
There was a problem hiding this comment.
But I can switch to UserException.MalformedFile all the same, if you prefer.
There was a problem hiding this comment.
Fixing htsjdk errors would be a nice benefit of redoing things in htsjdk, although it's going to be a big undertaking I think.
There was a problem hiding this comment.
How about I introduce a UserException.FailureToIndexFile exception here -- that would be accurate and address your concerns about tests erroneously passing.
There was a problem hiding this comment.
I'm actually more in favor of throwing the more general exception with the exposure of the underlying issue. It's just nasty for testing.
… a malformed file Resolves #4184
droazen
force-pushed
the
dr_indexfeaturefile_better_error_message_on_malformed_file
branch
from
f238f5a
to
4c2d044
Compare
|
@ldgauthier Introduced a suitably-vague |
|
👍 |
Codecov Report
@@ Coverage Diff @@
## master #4187 +/- ##
==============================================
- Coverage 78.45% 78.414% -0.036%
- Complexity 16636 16649 +13
==============================================
Files 1058 1058
Lines 59759 59928 +169
Branches 9746 9786 +40
==============================================
+ Hits 46881 46992 +111
- Misses 9101 9151 +50
- Partials 3777 3785 +8
|
droazen
deleted the
dr_indexfeaturefile_better_error_message_on_malformed_file
branch
Resolves #4184