biojava icon indicating copy to clipboard operation
biojava copied to clipboard
verified publisher icon biojava

PDBStatusTest might cause flakiness due to network calls

Open Shilpa-Rani opened this issue 5 years ago • 6 comments

The following functions in PDBStatus are called from PDBStatusTest.java: 1.public static Status getStatus(String pdbId) throws IOException 2.public static Status[] getStatus(String[] pdbIds) throws IOException 3.public static String getCurrent(String oldPdbId) throws IOException

But these function makes network calls, due to latency and network errors, these tests might fail sometimes, introducing flakiness. So, a better way to circumvent this issue is via mocking.

Example: testGetStatus() calls getStatus() in PDBStatus and a call is made to the following URL: https://data.rcsb.org/rest/v1/holdings/status/1HHB Assert.assertEquals(Status.REMOVED, PDBStatus.getStatus("1HHB"));

Considering the data returned from the call, the resulting JSON node can be marked as:

String jsonString = "{\"rcsb_repository_holdings_combined\":{\"id_code_replaced_by_latest\":\"4HHB\",\"status\":\"REMOVED\",\"status_code\":\"OBS\"},\"rcsb_repository_holdings_combined_entry_container_identifiers\":{\"entry_id\":\"1HHB\",\"rcsb_id\":\"1HHB\",\"update_id\":\"2020_48\"},\"rcsb_id\":\"1HHB\"}";
ObjectMapper mapper = new ObjectMapper();
JsonNode **actualObj** = mapper.readTree(jsonString);

The getStatus() function is:

	/**
	 * Get the status of a PDB id.
	 *
	 * @param pdbId the id
	 * @return The status.
	 */
	public static Status getStatus(String pdbId) throws IOException {
		URL url = new URL(String.format(STATUS_ENDPOINT, DEFAULT_RCSB_DATA_API_SERVER, pdbId.toUpperCase()));
		ObjectMapper objectMapper = new ObjectMapper();
		JsonNode node = objectMapper.readValue(url.openStream(), JsonNode.class);
		return parseStatusRecord(node);
	}

We can use power mockito dependency in POM.xml

		<!-- https://mvnrepository.com/artifact/org.mockito/mockito-all -->
		<dependency>
    		<groupId>org.mockito</groupId>
    		<artifactId>mockito-all</artifactId>
    		<version>2.0.2-beta</version>
    		<scope>test</scope>
		</dependency>

A few ways to mock the various objects are:

        ObjectMapper mockObjectMapper = Mockito.mock(ObjectMapper.class);
	InputStream inr = Mockito.mock(InputStream.class);
           
	when(mockObjectMapper.readValue(Mockito.any(InputStream.class), JsonNode.class)).thenReturn(**actualObj**);
	when(mockObjectMapper.readValue(Mockito.mock(InputStream.class), JsonNode.class)).thenReturn(**actualObj**);

This can be better achieved by moving parseStatusRecord() logic from getStatus() so that only the output from the network call can be mocked.

Shilpa-Rani avatar Dec 11 '20 19:12 Shilpa-Rani

Some of the other tests that failed due to network call flakiness in the builds since October 1st are:

Biojava-core:

Error: GenbankProxySequenceReaderTest.testFeatures:134 » NullPointer 
Error: GenbankProxySequenceReaderTest.testProteinSequenceFactoring:188 » NullPointer

Biojava-structure:

Error: TestDownloadChemCompProvider.testRedirectWorks:77 <NA>
Error: TestDSSPParser.testDSSPParser:60 » IO Server returned HTTP 
Error: PDBStatusTest.testGetReplacement:66 » NullPointer <NA>
Error: PDBStatusTest.testGetCurrent:79 » NullPointer <NA>
Error: PDBStatusTest.testGetReplaces:92 expected:<[1HHB]> but was:<null>

Other Reasons - Caused by: java.net.ConnectException:

Error: AtomCacheTest.testGetStructureForChainlessDomains:171 » ScopIO java.io.IOExcep... 
Error: AtomCacheTest.testGetStructureForDomain1:109 » ScopIO java.io.IOException: Una... 
Error: AtomCacheTest.testGetStructureForDomain2:128 » ScopIO java.io.IOException: Una... 
Error: AtomCacheTest.testGetStructureForDomain3:152 » ScopIO java.io.IOException: Una...

Biojava-integerationtest Error: TestQuatSymmetryDetectorExamples.testLocal:270 Stoichiometry A12B12C12D12E12H2 not expected for BIO:3JC9:1

Shilpa-Rani avatar Dec 11 '20 20:12 Shilpa-Rani

A few more tests that fail in biojava-structure because of Network Call are as follows:

[ERROR]   PDBStatusTest.testGetCurrent:89 » UnknownHost data.rcsb.org
[ERROR]   PDBStatusTest.testGetStatus:73 » UnknownHost data.rcsb.org
[ERROR]   PDBStatusTest.testGetStatusMultipleIds:81 » UnknownHost data.rcsb.org
[ERROR]   TestAtomCache.testFetchCurrent1HHB:192 » UnknownHost data.rcsb.org
[ERROR]   TestAtomCache.testFetchObsolete:209 » UnknownHost data.rcsb.org
[ERROR]   TestLoadStructureFromURL.testLoadStructureFromURL:57 » UnknownHost ftp.wwpdb.o...
[ERROR]   AtomCacheTest.testEmptyChemComp:396 » NoSuchFile /var/folders/gk/3g6b057n4j54y...
[ERROR]   AtomCacheTest.testEmptyGZChemComp:459 » NoSuchFile /var/folders/gk/3g6b057n4j5...
[ERROR]   AtomCacheTest.testFetchBehavior:229 » UnknownHost ftp.wwpdb.org
[ERROR]   TestNonDepositedFiles.testStructureWithBranchedEntities:452 » UnknownHost raw....
[ERROR]   TestURLBasedFileParsing.testMMcifURL:42 » UnknownHost ftp.wwpdb.org
[ERROR]   TestMmtfPerformance.test3HBX:77 » UnknownHost files.rcsb.org
[ERROR]   TestMmtfRoundTrip.testStructWithBranchedEntitiesRoundTrip:362 » UnknownHost ra...

Required Logs:

[ERROR] Tests run: 3, Failures: 0, Errors: 3, Skipped: 0, Time elapsed: 0.643 s <<< FAILURE! - in org.biojava.nbio.structure.PDBStatusTest
[ERROR] testGetStatusMultipleIds(org.biojava.nbio.structure.PDBStatusTest)  Time elapsed: 0.59 s  <<< ERROR!
java.net.UnknownHostException: data.rcsb.org
	at org.biojava.nbio.structure.PDBStatusTest.testGetStatusMultipleIds(PDBStatusTest.java:81)

[ERROR] testGetCurrent(org.biojava.nbio.structure.PDBStatusTest)  Time elapsed: 0.001 s  <<< ERROR!
java.net.UnknownHostException: data.rcsb.org
	at org.biojava.nbio.structure.PDBStatusTest.testGetCurrent(PDBStatusTest.java:89)

[ERROR] testGetStatus(org.biojava.nbio.structure.PDBStatusTest)  Time elapsed: 0.014 s  <<< ERROR!
java.net.UnknownHostException: data.rcsb.org
	at org.biojava.nbio.structure.PDBStatusTest.testGetStatus(PDBStatusTest.java:73)

...

[ERROR] Tests run: 6, Failures: 0, Errors: 2, Skipped: 0, Time elapsed: 1.009 s <<< FAILURE! - in org.biojava.nbio.structure.TestAtomCache
[ERROR] testFetchObsolete(org.biojava.nbio.structure.TestAtomCache)  Time elapsed: 0.277 s  <<< ERROR!
java.net.UnknownHostException: data.rcsb.org
	at org.biojava.nbio.structure.TestAtomCache.testFetchObsolete(TestAtomCache.java:209)

[ERROR] testFetchCurrent1HHB(org.biojava.nbio.structure.TestAtomCache)  Time elapsed: 0.002 s  <<< ERROR!
java.net.UnknownHostException: data.rcsb.org
	at org.biojava.nbio.structure.TestAtomCache.testFetchCurrent1HHB(TestAtomCache.java:192)

...

[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.399 s <<< FAILURE! - in org.biojava.nbio.structure.TestLoadStructureFromURL
[ERROR] testLoadStructureFromURL(org.biojava.nbio.structure.TestLoadStructureFromURL)  Time elapsed: 0.358 s  <<< ERROR!
java.net.UnknownHostException: ftp.wwpdb.org
	at org.biojava.nbio.structure.TestLoadStructureFromURL.testLoadStructureFromURL(TestLoadStructureFromURL.java:57)

...

[ERROR] Tests run: 10, Failures: 0, Errors: 3, Skipped: 0, Time elapsed: 2.072 s <<< FAILURE! - in org.biojava.nbio.structure.align.util.AtomCacheTest
[ERROR] testEmptyGZChemComp(org.biojava.nbio.structure.align.util.AtomCacheTest)  Time elapsed: 0.049 s  <<< ERROR!
java.nio.file.NoSuchFileException: /var/folders/gk/3g6b057n4j54yjjrfvjvns440000gn/T/BIOJAVA_TEST_CACHE/chemcomp/ATP.cif.gz
	at org.biojava.nbio.structure.align.util.AtomCacheTest.testEmptyGZChemComp(AtomCacheTest.java:459)

[ERROR] testFetchBehavior(org.biojava.nbio.structure.align.util.AtomCacheTest)  Time elapsed: 0.004 s  <<< ERROR!
java.net.UnknownHostException: ftp.wwpdb.org
	at org.biojava.nbio.structure.align.util.AtomCacheTest.testFetchBehavior(AtomCacheTest.java:229)

[ERROR] testEmptyChemComp(org.biojava.nbio.structure.align.util.AtomCacheTest)  Time elapsed: 0.008 s  <<< ERROR!
java.nio.file.NoSuchFileException: /var/folders/gk/3g6b057n4j54yjjrfvjvns440000gn/T/BIOJAVA_TEST_CACHE/chemcomp/ATP.cif.gz
	at org.biojava.nbio.structure.align.util.AtomCacheTest.testEmptyChemComp(AtomCacheTest.java:396)

...

[ERROR] Tests run: 13, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.856 s <<< FAILURE! - in org.biojava.nbio.structure.io.TestNonDepositedFiles
[ERROR] testStructureWithBranchedEntities(org.biojava.nbio.structure.io.TestNonDepositedFiles)  Time elapsed: 0.23 s  <<< ERROR!
java.net.UnknownHostException: raw.githubusercontent.com
	at org.biojava.nbio.structure.io.TestNonDepositedFiles.testStructureWithBranchedEntities(TestNonDepositedFiles.java:452)

...

[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.404 s <<< FAILURE! - in org.biojava.nbio.structure.io.TestURLBasedFileParsing
[ERROR] testMMcifURL(org.biojava.nbio.structure.io.TestURLBasedFileParsing)  Time elapsed: 0.365 s  <<< ERROR!
java.net.UnknownHostException: ftp.wwpdb.org
	at org.biojava.nbio.structure.io.TestURLBasedFileParsing.testMMcifURL(TestURLBasedFileParsing.java:42)

...

[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.545 s <<< FAILURE! - in org.biojava.nbio.structure.io.mmtf.TestMmtfPerformance
[ERROR] test3HBX(org.biojava.nbio.structure.io.mmtf.TestMmtfPerformance)  Time elapsed: 0.504 s  <<< ERROR!
java.net.UnknownHostException: files.rcsb.org
	at org.biojava.nbio.structure.io.mmtf.TestMmtfPerformance.test3HBX(TestMmtfPerformance.java:77)

...

[ERROR] Tests run: 2, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.135 s <<< FAILURE! - in org.biojava.nbio.structure.io.mmtf.TestMmtfRoundTrip
[ERROR] testStructWithBranchedEntitiesRoundTrip(org.biojava.nbio.structure.io.mmtf.TestMmtfRoundTrip)  Time elapsed: 0.215 s  <<< ERROR!
java.net.UnknownHostException: raw.githubusercontent.com
	at org.biojava.nbio.structure.io.mmtf.TestMmtfRoundTrip.testStructWithBranchedEntitiesRoundTrip(TestMmtfRoundTrip.java:362)

...

Shilpa-Rani avatar Dec 14 '20 02:12 Shilpa-Rani

A few more tests that fail due to Network Failure in biojava-core:

[ERROR]   SearchIOTest.testConstructorWithEvalueHspFilter:110 test failed: www.ncbi.nlm.nih.gov
[ERROR]   SearchIOTest.testConstructorWithoutFactoryGuess:92 test failed: www.ncbi.nlm.nih.gov
[ERROR]   BlastXMLParserTest.testCreateObjects:101 » UnknownHost www.ncbi.nlm.nih.gov
[ERROR]   GenbankCookbookTest.testProcess:75 » NullPointer
        [ERROR]   GenbankProxySequenceReaderTest.testFeatures:134 » NullPointer
        [ERROR]   GenbankProxySequenceReaderTest.testProteinSequenceFactoring:188 » NullPointer
	[ERROR]   GenbankProxySequenceReaderTest.testProteinSequenceFactoring:210

Required Logs:

...
[ERROR] Tests run: 3, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.396 s <<< FAILURE! - in org.biojava.nbio.core.search.io.SearchIOTest
[ERROR] testConstructorWithEvalueHspFilter(org.biojava.nbio.core.search.io.SearchIOTest)  Time elapsed: 0.339 s  <<< FAILURE!
java.lang.AssertionError: 
test failed:
www.ncbi.nlm.nih.gov
	at org.biojava.nbio.core.search.io.SearchIOTest.testConstructorWithEvalueHspFilter(SearchIOTest.java:110)

[ERROR] testConstructorWithoutFactoryGuess(org.biojava.nbio.core.search.io.SearchIOTest)  Time elapsed: 0.002 s  <<< FAILURE!
java.lang.AssertionError: 
test failed:
www.ncbi.nlm.nih.gov
	at org.biojava.nbio.core.search.io.SearchIOTest.testConstructorWithoutFactoryGuess(SearchIOTest.java:92)
...

[INFO] Running org.biojava.nbio.core.search.io.blast.BlastXMLParserTest
createObjects
getFileExtensions
setFile
[ERROR] Tests run: 6, Failures: 0, Errors: 1, Skipped: 2, Time elapsed: 0.549 s <<< FAILURE! - in org.biojava.nbio.core.search.io.blast.BlastXMLParserTest
[ERROR] testCreateObjects(org.biojava.nbio.core.search.io.blast.BlastXMLParserTest)  Time elapsed: 0.505 s  <<< ERROR!
java.net.UnknownHostException: www.ncbi.nlm.nih.gov
	at org.biojava.nbio.core.search.io.blast.BlastXMLParserTest.testCreateObjects(BlastXMLParserTest.java:101)
...

[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.569 s <<< FAILURE! - in org.biojava.nbio.core.sequence.io.GenbankCookbookTest
[ERROR] testProcess(org.biojava.nbio.core.sequence.io.GenbankCookbookTest)  Time elapsed: 0.212 s  <<< ERROR!
java.lang.NullPointerException
	at org.biojava.nbio.core.sequence.io.GenbankCookbookTest.testProcess(GenbankCookbookTest.java:75)

Shilpa-Rani avatar Dec 14 '20 02:12 Shilpa-Rani

A few more tests that fail in biojava-integrationtest because of Network Call are as follows:

[ERROR] Failures: 
[ERROR]   TestParsingScopDomainPDB.testd1gena:53 scop.berkeley.edu
[ERROR] Errors: 
[ERROR]   EcodInstallationTest.testDownloads:88 » UnknownHost prodata.swmed.edu
[ERROR]   CifFileSupplierIntegrationTest.test1A2C:47->testRoundTrip:52 » UnknownHost fil...
[ERROR]   CifFileSupplierIntegrationTest.test1SMT:31->testRoundTrip:52 » UnknownHost fil...
[ERROR]   CifFileSupplierIntegrationTest.test2N3J:41->testRoundTrip:52 » UnknownHost fil...

Shilpa-Rani avatar Dec 14 '20 03:12 Shilpa-Rani

Thanks for the list of network calls, @Shilpa-Rani.

I think we need to maintain some integration tests that include network calls so that we can detect changes in upstream data sources. However, I agree it would be nice if the main travis CI check was isolated from this so that builds don't fail immediately if the network goes down.

Is it possible to set up two travis processes that run for each commit? So for example the non-integration tests should be required to pass before merging, while the integration tests can sometimes be ignored if needed? Although I'm not sure whether that's really better...

sbliven avatar Jan 04 '21 10:01 sbliven

This pull request: https://github.com/biojava/biojava/pull/1022 fixes the issue by adding a Junit rule to intercept network errors during test execution and skips those tests.

unshorn avatar Feb 27 '22 03:02 unshorn

Is this issue still valid or we should close now? What about #1022 ?

aalhossary avatar May 09 '23 15:05 aalhossary

I think we should close this. It is true that it is not ideal that there's a dependency on network in some tests. We should strive to have all tests that depend on network in integration-test module.

Note: since transitioning to github workflows some time ago, we run integration-tests only in pull requests but not in master: see https://github.com/biojava/biojava/blob/88b51deb93758e556b231f8b438199559a9bc188/.github/workflows/master.yml#L36

josemduarte avatar May 17 '23 17:05 josemduarte