Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit Tests and fixes for a few classes in samtools.cram.io #1198

Merged
merged 7 commits into from
Oct 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions src/main/java/htsjdk/samtools/cram/io/CramInt.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class CramInt {
* @return an integer value read
* @throws IOException as per java IO contract
*/
public static int int32(final InputStream inputStream) throws IOException {
public static int readInt32(final InputStream inputStream) throws IOException {
return inputStream.read() | inputStream.read() << 8 | inputStream.read() << 16 | inputStream.read() << 24;
}

Expand All @@ -27,7 +27,7 @@ public static int int32(final InputStream inputStream) throws IOException {
* @param data input stream to read from
* @return an integer value read
*/
public static int int32(final byte[] data) {
public static int readInt32(final byte[] data) {
if (data.length != 4)
throw new IllegalArgumentException("Expecting a 4-byte integer. ");
return (0xFF & data[0]) | ((0xFF & data[1]) << 8) | ((0xFF & data[2]) << 16) | ((0xFF & data[3]) << 24);
Expand All @@ -39,8 +39,11 @@ public static int int32(final byte[] data) {
* @param buffer {@link ByteBuffer} to read from
* @return an integer value read from the buffer
*/
public static int int32(final ByteBuffer buffer) {
return buffer.get() | buffer.get() << 8 | buffer.get() << 16 | buffer.get() << 24;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These shifts were being attempted in a byte context

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a test case that fails without this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does my 0xff comment work for you on this point?

public static int readInt32(final ByteBuffer buffer) {
return (0xFF & buffer.get()) |
(0xFF & buffer.get()) << 8 |
(0xFF & buffer.get()) << 16 |
(0xFF & buffer.get()) << 24;
}

/**
Expand All @@ -61,7 +64,7 @@ public static int writeInt32(final int value, final OutputStream outputStream) t
}

/**
* Write int value to {@link OutputStream} encoded as CRAM int data type.
* Write int value to an array of bytes encoded as CRAM int data type.
*
* @param value value to be written out
* @return the byte array holding the value encoded as CRAM int data type
Expand All @@ -74,5 +77,4 @@ public static byte[] writeInt32(final int value) {
data[3] = (byte) (value >> 24 & 0xFF);
return data;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
/**
* Methods to read and write CRAM array of integers data type.
*/
public class CramArray {
public class CramIntArray {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed because this gave me the impression of a generic array type, though it's limited to integer. I can revert this if you prefer.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me.

/**
* Read CRAM int array from a {@link InputStream}.
*
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/htsjdk/samtools/cram/structure/Block.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public static Block readFromInputStream(final int major, InputStream inputStream
InputStreamUtils.readFully(inputStream, block.compressedContent, 0, block.compressedContent.length);
if (v3OrHigher) {
final int actualChecksum = ((CRC32InputStream) inputStream).getCRC32();
final int checksum = CramInt.int32(inputStream);
final int checksum = CramInt.readInt32(inputStream);
if (checksum != actualChecksum)
throw new RuntimeException(String.format("Block CRC32 mismatch: %04x vs %04x", checksum, actualChecksum));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package htsjdk.samtools.cram.structure;

import htsjdk.samtools.cram.io.CRC32OutputStream;
import htsjdk.samtools.cram.io.CramArray;
import htsjdk.samtools.cram.io.CramIntArray;
import htsjdk.samtools.cram.io.CramInt;
import htsjdk.samtools.cram.io.ITF8;
import htsjdk.samtools.cram.io.LTF8;
Expand Down Expand Up @@ -49,17 +49,17 @@ public boolean readContainerHeader(final int major, final Container container, f
peek[i] = (byte) character;
}

container.containerByteSize = CramInt.int32(peek);
container.containerByteSize = CramInt.readInt32(peek);
container.sequenceId = ITF8.readUnsignedITF8(inputStream);
container.alignmentStart = ITF8.readUnsignedITF8(inputStream);
container.alignmentSpan = ITF8.readUnsignedITF8(inputStream);
container.nofRecords = ITF8.readUnsignedITF8(inputStream);
container.globalRecordCounter = LTF8.readUnsignedLTF8(inputStream);
container.bases = LTF8.readUnsignedLTF8(inputStream);
container.blockCount = ITF8.readUnsignedITF8(inputStream);
container.landmarks = CramArray.array(inputStream);
container.landmarks = CramIntArray.array(inputStream);
if (major >= 3)
container.checksum = CramInt.int32(inputStream);
container.checksum = CramInt.readInt32(inputStream);

return true;
}
Expand All @@ -84,7 +84,7 @@ public int writeContainerHeader(final int major, final Container container, fina
length += (LTF8.writeUnsignedLTF8(container.globalRecordCounter, crc32OutputStream) + 7) / 8;
length += (LTF8.writeUnsignedLTF8(container.bases, crc32OutputStream) + 7) / 8;
length += (ITF8.writeUnsignedITF8(container.blockCount, crc32OutputStream) + 7) / 8;
length += (CramArray.write(container.landmarks, crc32OutputStream) + 7) / 8;
length += (CramIntArray.write(container.landmarks, crc32OutputStream) + 7) / 8;

if (major >= 3) {
outputStream.write(crc32OutputStream.getCrc32_LittleEndian());
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/htsjdk/samtools/cram/structure/SliceIO.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import htsjdk.samtools.SAMTagUtil;
import htsjdk.samtools.ValidationStringency;
import htsjdk.samtools.cram.common.CramVersions;
import htsjdk.samtools.cram.io.CramArray;
import htsjdk.samtools.cram.io.CramIntArray;
import htsjdk.samtools.cram.io.ITF8;
import htsjdk.samtools.cram.io.InputStreamUtils;
import htsjdk.samtools.cram.io.LTF8;
Expand Down Expand Up @@ -54,7 +54,7 @@ private static void parseSliceHeaderBlock(final int major, final Slice slice) th
slice.globalRecordCounter = LTF8.readUnsignedLTF8(inputStream);
slice.nofBlocks = ITF8.readUnsignedITF8(inputStream);

slice.contentIDs = CramArray.array(inputStream);
slice.contentIDs = CramIntArray.array(inputStream);
slice.embeddedRefBlockContentID = ITF8.readUnsignedITF8(inputStream);
slice.refMD5 = new byte[16];
InputStreamUtils.readFully(inputStream, slice.refMD5, 0, slice.refMD5.length);
Expand Down Expand Up @@ -85,7 +85,7 @@ private static byte[] createSliceHeaderBlockContent(final int major, final Slice
int i = 0;
for (final int id : slice.external.keySet())
slice.contentIDs[i++] = id;
CramArray.write(slice.contentIDs, byteArrayOutputStream);
CramIntArray.write(slice.contentIDs, byteArrayOutputStream);
ITF8.writeUnsignedITF8(slice.embeddedRefBlockContentID, byteArrayOutputStream);
byteArrayOutputStream.write(slice.refMD5 == null ? new byte[16] : slice.refMD5);

Expand Down
29 changes: 0 additions & 29 deletions src/test/java/htsjdk/cram/io/ExternalCompressionTest.java

This file was deleted.

4 changes: 2 additions & 2 deletions src/test/java/htsjdk/samtools/cram/VersionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public void test_V3() throws IOException {
CRC32 digester = new CRC32();
digester.update(containerHeaderBytes);
Assert.assertEquals(container.checksum, (int) digester.getValue());
Assert.assertEquals(CramInt.int32(crcBytes), container.checksum);
Assert.assertEquals(CramInt.readInt32(crcBytes), container.checksum);

// test block's crc:
cramSeekableStream.seek(firstBlockStart);
Expand All @@ -102,6 +102,6 @@ public void test_V3() throws IOException {
crcBytes = InputStreamUtils.readFully(cramSeekableStream, crcByteSize);
digester = new CRC32();
digester.update(blockBytes);
Assert.assertEquals(CramInt.int32(crcBytes), (int) digester.getValue());
Assert.assertEquals(CramInt.readInt32(crcBytes), (int) digester.getValue());
}
}
27 changes: 27 additions & 0 deletions src/test/java/htsjdk/samtools/cram/io/CramIntArrayTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package htsjdk.samtools.cram.io;

import htsjdk.HtsjdkTest;
import org.testng.Assert;
import org.testng.annotations.Test;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.List;

public class CramIntArrayTest extends HtsjdkTest {

@Test(dataProvider = "testInt32Arrays", dataProviderClass = IOTestCases.class)
public void runTest(List<Integer> ints) throws IOException {

int[] inputArray = ints.stream().mapToInt(Integer::intValue).toArray();
try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
CramIntArray.write(inputArray, baos);

try (ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray())) {
int[] outputArray = CramIntArray.array(bais);
Assert.assertEquals(inputArray, outputArray, "Arrays did not match");
}
}
}
}
106 changes: 106 additions & 0 deletions src/test/java/htsjdk/samtools/cram/io/CramIntTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package htsjdk.samtools.cram.io;

import htsjdk.HtsjdkTest;
import org.testng.Assert;
import org.testng.annotations.Test;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.List;

public class CramIntTest extends HtsjdkTest {
private byte[] streamWritten(List<Integer> ints) throws IOException {
try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
for (int value : ints) {
CramInt.writeInt32(value, baos);
}
return baos.toByteArray();
}
}

private byte[] byteArrayWritten(List<Integer> ints) {
final int bufSize = 4;
final int arraySize = bufSize * ints.size();
byte[] array = new byte[arraySize];

int offset = 0;
byte[] arrayBuffer;

for (int value : ints) {
arrayBuffer = CramInt.writeInt32(value);
System.arraycopy(arrayBuffer, 0, array, offset, bufSize);
offset += bufSize;
}

return array;
}

@Test(dataProvider = "littleEndianTests32", dataProviderClass = IOTestCases.class)
public void checkStreamLittleEndian(Integer testInt, byte[] expected) throws IOException {
List<Integer> ints = new ArrayList<>();
ints.add(testInt);

byte[] actual = streamWritten(ints);
Assert.assertEquals(actual, expected);
}

@Test(dataProvider = "littleEndianTests32", dataProviderClass = IOTestCases.class)
public void checkByteArrayLittleEndian(Integer testInt, byte[] expected) {
List<Integer> ints = new ArrayList<>();
ints.add(testInt);

byte[] actual = byteArrayWritten(ints);
Assert.assertEquals(actual, expected);
}

// Combinatorial tests of 2 CramInt write methods x 3 CramInt read methods

@Test(dataProvider = "testInt32Arrays", dataProviderClass = IOTestCases.class)
public void matchStreamRead(List<Integer> ints) throws IOException {
byte[][] inputs = {streamWritten(ints), byteArrayWritten(ints)};

for (byte[] byteArray : inputs) {
try (ByteArrayInputStream bais = new ByteArrayInputStream(byteArray)) {
for (int value : ints) {
int fromStream = CramInt.readInt32(bais);
Assert.assertEquals(fromStream, value, "Value did not match");
}
}
}
}

@Test(dataProvider = "testInt32Arrays", dataProviderClass = IOTestCases.class)
public void matchBufferRead(List<Integer> ints) throws IOException {
byte[][] inputs = {streamWritten(ints), byteArrayWritten(ints)};

for (byte[] byteArray : inputs) {
ByteBuffer bb = ByteBuffer.wrap(byteArray);

for (int value : ints) {
int fromBuffer = CramInt.readInt32(bb);
Assert.assertEquals(fromBuffer, value, "Value did not match");
}
}
}

@Test(dataProvider = "testInt32Arrays", dataProviderClass = IOTestCases.class)
public void matchByteArrayRead(List<Integer> ints) throws IOException {
byte[][] inputs = {streamWritten(ints), byteArrayWritten(ints)};

for (byte[] inputArray : inputs) {
final int bufSize = 4;
byte[] outBuf = new byte[bufSize];
int offset = 0;

for (int value : ints) {
System.arraycopy(inputArray, offset, outBuf, 0, bufSize);
int fromBuffer = CramInt.readInt32(outBuf);
Assert.assertEquals(fromBuffer, value, "Value did not match");
offset += bufSize;
}
}
}
}
54 changes: 54 additions & 0 deletions src/test/java/htsjdk/samtools/cram/io/ExternalCompressionTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package htsjdk.samtools.cram.io;

import htsjdk.HtsjdkTest;
import htsjdk.samtools.cram.encoding.rans.RANS;
import org.testng.Assert;
import org.testng.annotations.Test;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;

public class ExternalCompressionTest extends HtsjdkTest {
public static final File BZIP2_FILE = new File("src/test/resources/htsjdk/samtools/cram/io/bzip2-test.bz2");
public static final byte[] TEST_BYTES = "This is a simple string to test compression".getBytes();

@Test
public void testBZip2Decompression() throws IOException {
final byte [] input = Files.readAllBytes(BZIP2_FILE.toPath());
final byte [] output = ExternalCompression.unbzip2(input);
Assert.assertEquals(output, "BZip2 worked".getBytes());
}

@Test
public void testGZipRoundtrip() throws IOException {
final byte [] compressed = ExternalCompression.gzip(TEST_BYTES);
final byte [] restored = ExternalCompression.gunzip(compressed);
Assert.assertEquals(TEST_BYTES, restored);
}

@Test
public void testBZip2Roundtrip() throws IOException {
final byte [] compressed = ExternalCompression.bzip2(TEST_BYTES);
final byte [] restored = ExternalCompression.unbzip2(compressed);
Assert.assertEquals(TEST_BYTES, restored);
}

@Test
public void testRANSRoundtrip() {
for(RANS.ORDER order : RANS.ORDER.values()) {
final byte[] compressed = ExternalCompression.rans(TEST_BYTES, order);
final byte[] restored = ExternalCompression.unrans(compressed);
Assert.assertEquals(TEST_BYTES, restored);
}
}

@Test
public void testXZRoundtrip() throws IOException {
final byte [] compressed = ExternalCompression.xz(TEST_BYTES);
final byte [] restored = ExternalCompression.unxz(compressed);
Assert.assertEquals(TEST_BYTES, restored);
}


}
Loading