Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Add file name to exception messages #750
Conversation
yfarjoun
was assigned
by ronlevine
Nov 22, 2016
|
@yfarjoun Please review. |
coveralls
commented
Nov 22, 2016
| @@ -283,7 +288,8 @@ public int read(final byte[] buffer, int offset, int length) | ||
| public void seek(final long pos) | ||
| throws IOException { | ||
| if (mFile == null) { | ||
| - throw new IOException("Cannot seek on stream based file"); | ||
| + | ||
| + throw new IOException("Cannot seek on stream based file " + mFile.getSource()); |
ronlevine
Nov 22, 2016
Contributor
Done. Cannot get the mFile path since mFile is null. The path is not accessible form mStream.
| @@ -302,7 +308,7 @@ public void seek(final long pos) | ||
| } | ||
| if (uncompressedOffset > available || | ||
| (uncompressedOffset == available && !eof())) { | ||
| - throw new IOException("Invalid file pointer: " + pos); | ||
| + throw new IOException("Invalid file pointer: " + pos + " for " + mFile.getSource()); |
|
@yfarjoun Responded to comments plus did some code cleanup. Please review. Note that I have not squashed/rebased. |
coveralls
commented
Nov 22, 2016
| @@ -77,10 +80,81 @@ public void testBasic() throws Exception { | ||
| } | ||
| @Test | ||
| - public void testOverflow() throws Exception { | ||
| + public void testTruncatedFile() throws Exception { |
yfarjoun
Nov 22, 2016
Contributor
these tests have very similar structure, could you refactor them into a single test with a DataProvider?
| @@ -77,10 +80,81 @@ public void testBasic() throws Exception { | ||
| } | ||
| @Test | ||
| - public void testOverflow() throws Exception { | ||
| + public void testTruncatedFile() throws Exception { | ||
| + final File f = new File("src/test/resources/htsjdk/tribble/vcfexample.vcf.truncated.gz"); |
yfarjoun
Nov 22, 2016
Contributor
extract "src/test/resources/htsjdk/tribble/" as a local private static
|
@yfarjoun Refactored tests using a Dataprovider and make a static final variable for the resources directory. Please review. |
coveralls
commented
Nov 23, 2016
coveralls
commented
Nov 23, 2016
| @@ -50,6 +50,13 @@ | ||
| * c.f. http://samtools.sourceforge.net/SAM1.pdf for details of BGZF format | ||
| */ | ||
| public class BlockCompressedInputStream extends InputStream implements LocationAware { | ||
| + | ||
| + public final static String INCORRECT_HEADER_SIZE_MSG = "Incorrect header size for "; |
| + | ||
| + public final static String INCORRECT_HEADER_SIZE_MSG = "Incorrect header size for "; | ||
| + public final static String UNEXPECTED_BLOCK_LENGTH_MSG = "Unexpected compressed block length: "; | ||
| + public final static String PREMATURE_END_MSG = "Premature end of "; |
| + public final static String INCORRECT_HEADER_SIZE_MSG = "Incorrect header size for "; | ||
| + public final static String UNEXPECTED_BLOCK_LENGTH_MSG = "Unexpected compressed block length: "; | ||
| + public final static String PREMATURE_END_MSG = "Premature end of "; | ||
| + public final static String CANNOT_SEEK_STREAM_MSG = "Cannot seek on stream based file "; |
ronlevine
Nov 23, 2016
•
Contributor
Don't want a colon because this message will not be followed by a file name, the file name is not accessible from the stream. I will remove the last space in the string.
yfarjoun
approved these changes
Nov 23, 2016
I'm happy now. please respond to @vdauwera but
|
|
coveralls
commented
Nov 23, 2016
ronlevine commentedNov 22, 2016
Description
Implements #747.
Added file name to exception messages so they are more informative.
Checklist