From a060fd030e571c1c8bd68fb53b0faf7c5ddcb5b7 Mon Sep 17 00:00:00 2001 From: Andrii Nikitiuk Date: Tue, 7 Jun 2016 19:30:00 -0400 Subject: [PATCH 1/2] Fixed SRA tests issues, corrected error message --- build.gradle | 1 + src/main/java/htsjdk/samtools/SRAFileReader.java | 3 +- .../java/htsjdk/samtools/sra/SRAAccession.java | 4 +- .../java/htsjdk/samtools/sra/AbstractSRATest.java | 20 ++++++ .../java/htsjdk/samtools/sra/SRAAccessionTest.java | 3 +- .../java/htsjdk/samtools/sra/SRAIndexTest.java | 2 +- .../htsjdk/samtools/sra/SRALazyRecordTest.java | 2 +- src/test/java/htsjdk/samtools/sra/SRATest.java | 78 +++++++++++----------- 8 files changed, 69 insertions(+), 44 deletions(-) diff --git a/build.gradle b/build.gradle index 174b702cd..9e8f35154 100644 --- a/build.gradle +++ b/build.gradle @@ -129,6 +129,7 @@ task testSRA(type: Test) { description "Run the SRA tests" useTestNG { + configFailurePolicy 'continue' includeGroups "sra" } } diff --git a/src/main/java/htsjdk/samtools/SRAFileReader.java b/src/main/java/htsjdk/samtools/SRAFileReader.java index 6925ffc8a..e76e10b0d 100644 --- a/src/main/java/htsjdk/samtools/SRAFileReader.java +++ b/src/main/java/htsjdk/samtools/SRAFileReader.java @@ -61,7 +61,8 @@ public SRAFileReader(final SRAAccession acc) { this.acc = acc; if (!acc.isValid()) { - throw new IllegalArgumentException("Invalid SRA accession was passed to SRA reader: " + acc); + throw new IllegalArgumentException("SRAFileReader: cannot resolve SRA accession '" + acc + "'\n" + + "Possible causes are an invalid SRA accession or a connection problem."); } try { diff --git a/src/main/java/htsjdk/samtools/sra/SRAAccession.java b/src/main/java/htsjdk/samtools/sra/SRAAccession.java index aadfdc336..17180d771 100644 --- a/src/main/java/htsjdk/samtools/sra/SRAAccession.java +++ b/src/main/java/htsjdk/samtools/sra/SRAAccession.java @@ -54,6 +54,8 @@ private final static String defaultAppVersionString = "[unknown software]"; private final static String htsJdkVersionString = "HTSJDK-NGS"; + static final String REMOTE_ACCESSION_PATTERN = "^[SED]RR[0-9]{6,9}$"; + private String acc; static { @@ -127,7 +129,7 @@ public static boolean isValid(String acc) { // anything else local other than a file is not an SRA archive looksLikeSRA = false; } else { - looksLikeSRA = acc.toUpperCase().matches ( "^[SED]RR[0-9]{6,9}$" ); + looksLikeSRA = acc.toUpperCase().matches ( REMOTE_ACCESSION_PATTERN ); } if (!looksLikeSRA) return false; diff --git a/src/test/java/htsjdk/samtools/sra/AbstractSRATest.java b/src/test/java/htsjdk/samtools/sra/AbstractSRATest.java index c50f3b84b..c7471cdf4 100644 --- a/src/test/java/htsjdk/samtools/sra/AbstractSRATest.java +++ b/src/test/java/htsjdk/samtools/sra/AbstractSRATest.java @@ -7,6 +7,7 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import java.lang.reflect.Method; import java.util.NoSuchElementException; @Test(groups = "sra") @@ -19,6 +20,25 @@ public final void assertSRAIsSupported(){ } } + @BeforeMethod + public final void skipIfCantResolve(Method method, Object[] params) { + String accession = null; + + if (params.length > 0) { + Object firstParam = params[0]; + if (firstParam instanceof String) { + accession = (String)firstParam; + } else if (firstParam instanceof SRAAccession) { + accession = firstParam.toString(); + } + } + + if (accession != null && + accession.matches(SRAAccession.REMOTE_ACCESSION_PATTERN) && !SRAAccession.isValid(accession)) { + throw new SkipException("Skipping network SRA Test because cannot resolve SRA accession."); + } + } + /** * Exhaust the iterator and check that it produce the expected number of mapped and unmapped reads. * Also checks that the hasNext() agrees with the actual results of next() for the given iterator. diff --git a/src/test/java/htsjdk/samtools/sra/SRAAccessionTest.java b/src/test/java/htsjdk/samtools/sra/SRAAccessionTest.java index e241ca941..4b89b7e52 100644 --- a/src/test/java/htsjdk/samtools/sra/SRAAccessionTest.java +++ b/src/test/java/htsjdk/samtools/sra/SRAAccessionTest.java @@ -13,8 +13,7 @@ private Object[][] getIsValidAccData() { return new Object[][] { { "SRR000123", true }, - { "DRR000001", true }, - { "SRR000000", false }, + { "DRR010511", true }, { "src/test/resources/htsjdk/samtools/sra/test_archive.sra", true }, { "src/test/resources/htsjdk/samtools/compressed.bam", false }, { "src/test/resources/htsjdk/samtools/uncompressed.sam", false }, diff --git a/src/test/java/htsjdk/samtools/sra/SRAIndexTest.java b/src/test/java/htsjdk/samtools/sra/SRAIndexTest.java index a14120334..0cdfc6915 100644 --- a/src/test/java/htsjdk/samtools/sra/SRAIndexTest.java +++ b/src/test/java/htsjdk/samtools/sra/SRAIndexTest.java @@ -46,7 +46,7 @@ * Created by andrii.nikitiuk on 10/28/15. */ public class SRAIndexTest extends AbstractSRATest { - private static final SRAAccession DEFAULT_ACCESSION = new SRAAccession("SRR1298981"); + private static final SRAAccession DEFAULT_ACCESSION = new SRAAccession("SRR2096940"); private static final int LAST_BIN_LEVEL = GenomicIndexUtil.LEVEL_STARTS.length - 1; private static final int SRA_BIN_OFFSET = GenomicIndexUtil.LEVEL_STARTS[LAST_BIN_LEVEL]; diff --git a/src/test/java/htsjdk/samtools/sra/SRALazyRecordTest.java b/src/test/java/htsjdk/samtools/sra/SRALazyRecordTest.java index 97a1ad863..36ef34681 100644 --- a/src/test/java/htsjdk/samtools/sra/SRALazyRecordTest.java +++ b/src/test/java/htsjdk/samtools/sra/SRALazyRecordTest.java @@ -12,7 +12,7 @@ * Tests for SRA extension of SAMRecord objects which load fields on demand */ public class SRALazyRecordTest extends AbstractSRATest { - private static final SRAAccession DEFAULT_ACCESSION = new SRAAccession("SRR1298981"); + private static final SRAAccession DEFAULT_ACCESSION = new SRAAccession("SRR2096940"); @DataProvider(name = "serializationTestData") private Object[][] getSerializationTestData() { diff --git a/src/test/java/htsjdk/samtools/sra/SRATest.java b/src/test/java/htsjdk/samtools/sra/SRATest.java index 2bdd7d724..c106bfc4a 100644 --- a/src/test/java/htsjdk/samtools/sra/SRATest.java +++ b/src/test/java/htsjdk/samtools/sra/SRATest.java @@ -101,13 +101,8 @@ public void testCountsBySpan(String acc, List chunks, int expectedNumMapp @DataProvider(name = "testGroups") private Object[][] createDataForGroups() { return new Object[][] { - {"SRR822962", new TreeSet<>(Arrays.asList( - "GS54389-FS3-L08", "GS57511-FS3-L08", "GS54387-FS3-L02", "GS54387-FS3-L01", - "GS57510-FS3-L01", "GS57510-FS3-L03", "GS54389-FS3-L07", "GS54389-FS3-L05", - "GS54389-FS3-L06", "GS57510-FS3-L02", "GS57510-FS3-L04", "GS54387-FS3-L03", - "GS46253-FS3-L03")) - }, - {"SRR2096940", new HashSet<>(Arrays.asList("SRR2096940"))} + {"SRR1035115", new TreeSet<>(Arrays.asList("15656144_B09YG", "15656144_B09MR"))}, + {"SRR2096940", new TreeSet<>(Arrays.asList("SRR2096940"))} }; } @@ -148,15 +143,17 @@ public void testGroups(String acc, Set groups) { private Object[][] createDataForReferences() { return new Object[][] { // primary alignment only - {"SRR1063272", 1, - Arrays.asList("supercont2.1", "supercont2.2", "supercont2.3", "supercont2.4", - "supercont2.5", "supercont2.6", "supercont2.7", "supercont2.8", - "supercont2.9", "supercont2.10", "supercont2.11", "supercont2.12", - "supercont2.13", "supercont2.14"), - Arrays.asList(2291499, 1621675, 1575141, 1084805, - 1814975, 1422463, 1399503, 1398693, - 1186808, 1059964, 1561994, 774062, - 756744, 926563)}, + {"SRR353866", 9, + Arrays.asList( + "AAAB01001871.1", "AAAB01002233.1", "AAAB01004056.1", "AAAB01006027.1", + "AAAB01008846.1", "AAAB01008859.1", "AAAB01008960.1", "AAAB01008982.1", + "AAAB01008987.1" + ), + Arrays.asList( + 1115, 1034, 1301, 1007, + 11308833, 12516315, 23099915, 1015562, + 16222597 + )}, }; } @@ -208,67 +205,66 @@ public void testReferences(String acc, int numberFirstReferenceFound, List Date: Wed, 13 Jul 2016 13:47:56 -0400 Subject: [PATCH 2/2] Network related tests now depend on ability to resolve single SRR000123 accession --- src/test/java/htsjdk/samtools/sra/AbstractSRATest.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/test/java/htsjdk/samtools/sra/AbstractSRATest.java b/src/test/java/htsjdk/samtools/sra/AbstractSRATest.java index c7471cdf4..a0984d75e 100644 --- a/src/test/java/htsjdk/samtools/sra/AbstractSRATest.java +++ b/src/test/java/htsjdk/samtools/sra/AbstractSRATest.java @@ -4,6 +4,7 @@ import htsjdk.samtools.SAMRecordIterator; import org.testng.Assert; import org.testng.SkipException; +import org.testng.annotations.BeforeGroups; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -12,9 +13,19 @@ @Test(groups = "sra") public abstract class AbstractSRATest { + private static boolean canResolveNetworkAccession = false; + private static String checkAccession = "SRR000123"; + + @BeforeGroups(groups = "sra") + public final void checkIfCanResolve() { + if (!SRAAccession.isSupported()) { + return; + } + canResolveNetworkAccession = SRAAccession.isValid(checkAccession); + } @BeforeMethod - public final void assertSRAIsSupported(){ + public final void assertSRAIsSupported() { if(!SRAAccession.isSupported()){ throw new SkipException("Skipping SRA Test because SRA native code is unavailable."); } @@ -34,8 +45,9 @@ public final void skipIfCantResolve(Method method, Object[] params) { } if (accession != null && - accession.matches(SRAAccession.REMOTE_ACCESSION_PATTERN) && !SRAAccession.isValid(accession)) { - throw new SkipException("Skipping network SRA Test because cannot resolve SRA accession."); + accession.matches(SRAAccession.REMOTE_ACCESSION_PATTERN) && !canResolveNetworkAccession) { + throw new SkipException("Skipping network SRA Test because cannot resolve remote SRA accession '" + + checkAccession + "'."); } }