Skip to content

Commit 2ad8e09

Browse files
aalhossaryaalhossary
committed
Test every ECOD layout offline
The distribution is 657 MB, so the only test that touched the current release was testVersion, which asserts on one string. Every other test pins develop204. That is why a wholesale format change surfaced as a single one-line assertion failure, eight months after it happened. EcodParserTest reads fixtures taken verbatim from the real files - 13, 15, 16, 23 and 25 columns, both version comment forms, a row ending in an empty column, an AlphaFold-derived row, and a malformed row - through the Reader constructor. Fifteen tests, no network, well under a second. testVersion additionally parses the first few thousand lines of the local file. That is enough to notice a column change without building the three million domains the whole file now holds.
1 parent 6cb9633 commit 2ad8e09

2 files changed

Lines changed: 363 additions & 0 deletions

File tree

‎biojava-integrationtest/src/test/java/org/biojava/nbio/structure/test/ecod/EcodInstallationTest.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,12 @@
2222

2323
import static org.junit.Assert.*;
2424

25+
import java.io.BufferedReader;
2526
import java.io.File;
27+
import java.io.FileReader;
2628
import java.io.IOException;
29+
import java.io.Reader;
30+
import java.io.StringReader;
2731
import java.util.ArrayList;
2832
import java.util.Arrays;
2933
import java.util.Collections;
@@ -47,6 +51,7 @@
4751
import org.biojava.nbio.structure.ecod.EcodDomain;
4852
import org.biojava.nbio.structure.ecod.EcodFactory;
4953
import org.biojava.nbio.structure.ecod.EcodInstallation;
54+
import org.biojava.nbio.structure.ecod.EcodInstallation.EcodParser;
5055
import org.junit.Ignore;
5156
import org.junit.Rule;
5257
import org.junit.Test;
@@ -277,12 +282,47 @@ public void testFilterByHierarchy() throws IOException {
277282
assertEquals(expected,actual);
278283
}
279284

285+
/**
286+
* Checks that the current release can still be read.
287+
* <p>
288+
* The version is read from the file's header without parsing the domains, so this
289+
* additionally parses the first few thousand lines of the same file. That is enough to
290+
* notice a column change — which is what ECOD did at v294.1, unnoticed for months —
291+
* without building the three million domains the whole file now holds.
292+
*/
280293
@Test
281294
public void testVersion() throws IOException {
282295
EcodDatabase ecod3 = EcodFactory.getEcodDatabase("latest");
283296
String version = ecod3.getVersion();
284297
assertNotNull(version);
285298
assertNotEquals("latest", version);
299+
System.out.println("latest version of ECOD is "+version);
300+
301+
File domainsFile = new File(((EcodInstallation) ecod3).getCacheLocation(),
302+
"ecod.latest.domains.txt");
303+
assertTrue("No local copy of the domains file at "+domainsFile, domainsFile.exists());
304+
305+
EcodParser parser = new EcodParser(firstLines(domainsFile, 5000));
306+
assertEquals(version, parser.getVersion());
307+
assertFalse("No domains parsed from ECOD "+version
308+
+ "; the distribution format has probably changed",
309+
parser.getDomains().isEmpty());
310+
}
311+
312+
/**
313+
* @return a reader over the first {@code maxLines} lines of the file
314+
*/
315+
private static Reader firstLines(File f, int maxLines) throws IOException {
316+
StringBuilder head = new StringBuilder();
317+
try (BufferedReader in = new BufferedReader(new FileReader(f))) {
318+
String line;
319+
int n = 0;
320+
while (n < maxLines && (line = in.readLine()) != null) {
321+
head.append(line).append('\n');
322+
n++;
323+
}
324+
}
325+
return new StringReader(head.toString());
286326
}
287327

288328
/**

‎biojava-structure/src/test/java/org/biojava/nbio/structure/ecod/EcodParserTest.java‎

Lines changed: 323 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,323 @@
1+
/*
2+
* BioJava development code
3+
*
4+
* This code may be freely distributed and modified under the
5+
* terms of the GNU Lesser General Public Licence. This should
6+
* be distributed with the code. If you do not have a copy,
7+
* see:
8+
*
9+
* http://www.gnu.org/copyleft/lesser.html
10+
*
11+
* Copyright for this code is held jointly by the individual
12+
* authors. These should be listed in @author doc comments.
13+
*
14+
* For more information on the BioJava project and its aims,
15+
* or to join the biojava-l mailing list, visit the home page
16+
* at:
17+
*
18+
* http://www.biojava.org/
19+
*/
20+
package org.biojava.nbio.structure.ecod;
21+
22+
import static org.junit.jupiter.api.Assertions.assertEquals;
23+
import static org.junit.jupiter.api.Assertions.assertFalse;
24+
import static org.junit.jupiter.api.Assertions.assertNull;
25+
import static org.junit.jupiter.api.Assertions.assertTrue;
26+
27+
import java.io.IOException;
28+
import java.io.StringReader;
29+
import java.util.Arrays;
30+
import java.util.Collections;
31+
import java.util.LinkedHashSet;
32+
import java.util.List;
33+
34+
import org.biojava.nbio.structure.ecod.EcodInstallation.EcodParser;
35+
import org.junit.jupiter.api.Nested;
36+
import org.junit.jupiter.api.Test;
37+
38+
/**
39+
* Checks that {@link EcodParser} reads every layout ECOD has distributed.
40+
* <p>
41+
* The columns have been renamed, reordered and added to several times, and the version
42+
* comment itself changed form at v294.1. Because the full distribution is 657 MB, none of
43+
* that was covered by a test that could run in reasonable time, and a format change went
44+
* unnoticed for months. These fixtures are taken verbatim from the real files, so the
45+
* contract is pinned in milliseconds rather than by a download.
46+
*
47+
* @author Amr ALHOSSARY
48+
* @since 7.3.0
49+
*/
50+
class EcodParserTest {
51+
52+
/** develop204, list format 1.5: 15 columns, commented header, quoted names. */
53+
private static final String DEVELOP204 = String.join("\n",
54+
"#/data/ecod/database_versions/v204/ecod.develop204.domains.txt",
55+
"#ECOD version develop204",
56+
"#Domain list version 1.5",
57+
"#Grishin lab (http://prodata.swmed.edu/ecod)",
58+
"#uid\tecod_domain_id\tmanual_rep\tf_id\tpdb\tchain\tpdb_range\tseqid_range"
59+
+ "\tarch_name\tx_name\th_name\tt_name\tf_name\tasm_status\tligand",
60+
"002137905\te6b4nA1\tAUTO_NONREP\t1.1.1\t6b4n\tA\tA:1-99\tA:1-99\tbeta barrels"
61+
+ "\t\"cradle loop barrel\"\t\"RIFT-related\"\t\"acid protease\""
62+
+ "\tF_UNCLASSIFIED\tNOT_DOMAIN_ASSEMBLY\tCL,G53,NA");
63+
64+
/** develop291, list format 1.6: 16 columns, f_id renamed t_id, unp_acc inserted at 9. */
65+
private static final String DEVELOP291 = String.join("\n",
66+
"#/data/ecod/database_versions/v291/ecod.develop291.domains.txt",
67+
"#ECOD version develop291",
68+
"#Domain list version 1.6",
69+
"#Grishin lab (http://prodata.swmed.edu/ecod)",
70+
"#uid\tecod_domain_id\tmanual_rep\tt_id\tpdb\tchain\tpdb_range\tseqid_range\tunp_acc"
71+
+ "\tarch_name\tx_name\th_name\tt_name\tf_name\tasm_status\tligand",
72+
"000000267\te1udzA1\tMANUAL_REP\t1.1.1\t1udz\tA\tA:203-381\tA:4-182\tP12345"
73+
+ "\tbeta barrels\t\"cradle loop barrel\"\t\"RIFT-related\"\t\"acid protease\""
74+
+ "\tF_UNCLASSIFIED\tNOT_DOMAIN_ASSEMBLY\tNO_LIGANDS_4A");
75+
76+
private static final String V295_COLUMNS =
77+
"uid\tecod_domain_id\tmanual_rep\tf_id\tpdb\tchain\tpdb_range\tseqid_range"
78+
+ "\tarchitecture_name\tx_name\th_name\tt_name\tf_name\tassembly_id\tdomain_id_short"
79+
+ "\trange_count\tarch_manual\tx_manual\th_manual\tt_manual\tf_manual"
80+
+ "\tvalid_structure\tligand_binding\tligand_comp_ids\tligand_pdbnum";
81+
82+
/** v295: 25 columns, uncommented header, True/False, empty assembly_id, moved ligands. */
83+
private static final String V295 = String.join("\n",
84+
"# ECOD Domain List",
85+
"# Version: v295",
86+
"# Generated: 2026-06-24 22:47:42",
87+
"# Ligand cutoff: 4.0 A (NO_LIGANDS_4A = no contact within cutoff)",
88+
"#",
89+
V295_COLUMNS,
90+
"0\te2nmzA1\tTrue\t1.1.1.3\t2nmz\tA\tA:1-99\tA:1-99\tbeta barrels\tcradle loop barrel"
91+
+ "\tRIFT-related\tacid protease\tRVP\t\t\t1\tFalse\tFalse\tFalse\tFalse\tTrue"
92+
+ "\tTrue\tTrue\tROC,SO4\tA:601,A:602,B:401",
93+
// the last column is empty on four rows in five, so split() must keep it
94+
"3\te2rspA1\tTrue\t1.1.1.3\t2rsp\tA\tA:1-124\tA:1-124\tbeta barrels\tcradle loop barrel"
95+
+ "\tRIFT-related\tacid protease\tRVP\t\t\t1\tFalse\tFalse\tFalse\tFalse\tTrue"
96+
+ "\tTrue\tFalse\tNO_LIGANDS_4A\t",
97+
// a domain classified from an AlphaFold model: no PDB entry, so no EcodDomain
98+
"3163557\tP44140_F1_nD2\tFalse\t2004.1.1.123\t\t\t131-315\t131-315\talpha bundles"
99+
+ "\tsomething\tsomething else\ta third thing\t\t\t\t1\tFalse\tFalse\tFalse"
100+
+ "\tFalse\tTrue\tTrue\tFalse\tNO_LIGANDS_4A\t");
101+
102+
private static List<EcodDomain> parse(String contents) throws IOException {
103+
return new EcodParser(new StringReader(contents)).getDomains();
104+
}
105+
106+
private static String version(String contents) throws IOException {
107+
return new EcodParser(new StringReader(contents)).getVersion();
108+
}
109+
110+
@Nested
111+
class Version {
112+
@Test
113+
void oldHeaderForm() throws IOException {
114+
assertEquals("develop204", version(DEVELOP204));
115+
assertEquals("develop291", version(DEVELOP291));
116+
}
117+
118+
@Test
119+
void newHeaderForm() throws IOException {
120+
assertEquals("v295", version(V295));
121+
assertEquals("v294.1", version("# ECOD Domain List\n# Version: v294.1\n"));
122+
}
123+
124+
@Test
125+
void listFormatVersionIsNotTheEcodVersion() throws IOException {
126+
// "#Domain list version 1.5" describes the columns, not the release
127+
assertNull(version("#Grishin lab\n#Domain list version 1.5\n"));
128+
}
129+
130+
@Test
131+
void absentVersionIsNull() throws IOException {
132+
assertNull(version("#Grishin lab (http://prodata.swmed.edu/ecod)\n"));
133+
}
134+
}
135+
136+
@Nested
137+
class OldFormats {
138+
@Test
139+
void listFormat15() throws IOException {
140+
List<EcodDomain> domains = parse(DEVELOP204);
141+
assertEquals(1, domains.size());
142+
EcodDomain d = domains.get(0);
143+
assertEquals(Long.valueOf(2137905), d.getUid());
144+
assertEquals("e6b4nA1", d.getDomainId());
145+
assertEquals(Boolean.FALSE, d.getManual());
146+
assertEquals(Integer.valueOf(1), d.getXGroup());
147+
assertEquals(Integer.valueOf(1), d.getHGroup());
148+
assertEquals(Integer.valueOf(1), d.getTGroup());
149+
assertNull(d.getFGroup());
150+
assertEquals("6B4N", d.getPdbId().getId());
151+
assertEquals("A", d.getChainId());
152+
assertEquals("A:1-99", d.getRange());
153+
assertEquals("A:1-99", d.getSeqIdRange());
154+
assertEquals("beta barrels", d.getArchitectureName());
155+
// quotes were stripped up to develop292
156+
assertEquals("cradle loop barrel", d.getXGroupName());
157+
assertEquals("RIFT-related", d.getHGroupName());
158+
assertEquals("acid protease", d.getTGroupName());
159+
assertEquals("F_UNCLASSIFIED", d.getFGroupName());
160+
assertEquals(Long.valueOf(2137905), d.getAssemblyId());
161+
assertEquals(new LinkedHashSet<>(Arrays.asList("CL", "G53", "NA")), d.getLigands());
162+
}
163+
164+
/**
165+
* develop291 inserted unp_acc before arch_name. Read positionally, every field from
166+
* there on shifts by one and the domain is silently mangled or dropped.
167+
*/
168+
@Test
169+
void listFormat16InsertsUnpAcc() throws IOException {
170+
List<EcodDomain> domains = parse(DEVELOP291);
171+
assertEquals(1, domains.size());
172+
EcodDomain d = domains.get(0);
173+
assertEquals(Boolean.TRUE, d.getManual());
174+
assertEquals("1UDZ", d.getPdbId().getId());
175+
assertEquals("A:4-182", d.getSeqIdRange());
176+
assertEquals("beta barrels", d.getArchitectureName());
177+
assertEquals("acid protease", d.getTGroupName());
178+
assertEquals("F_UNCLASSIFIED", d.getFGroupName());
179+
assertEquals(Long.valueOf(267), d.getAssemblyId());
180+
assertEquals(Collections.emptySet(), d.getLigands());
181+
}
182+
183+
/**
184+
* Headers were only added in develop101. Older files are still read by position.
185+
*/
186+
@Test
187+
void thirteenColumnsWithoutAHeader() throws IOException {
188+
List<EcodDomain> domains = parse(String.join("\n",
189+
"#ECOD version develop45",
190+
"000000001\te1udzA1\t1.1.1\t1udz\tA\tA:203-381\tbeta barrels"
191+
+ "\t\"cradle loop barrel\"\t\"RIFT-related\"\t\"acid protease\""
192+
+ "\tF_UNCLASSIFIED\tNOT_DOMAIN_ASSEMBLY\tNO_LIGANDS_4A"));
193+
assertEquals(1, domains.size());
194+
EcodDomain d = domains.get(0);
195+
assertNull(d.getManual(), "no manual_rep column before list format 1.1");
196+
assertNull(d.getSeqIdRange(), "no seqid_range column before list format 1.4");
197+
assertEquals("1UDZ", d.getPdbId().getId());
198+
assertEquals("acid protease", d.getTGroupName());
199+
}
200+
}
201+
202+
@Nested
203+
class NewFormat {
204+
@Test
205+
void twentyFiveColumns() throws IOException {
206+
List<EcodDomain> domains = parse(V295);
207+
// two PDB domains; the AlphaFold-derived row cannot be an EcodDomain
208+
assertEquals(2, domains.size());
209+
210+
EcodDomain d = domains.get(0);
211+
assertEquals(Long.valueOf(0), d.getUid());
212+
assertEquals("e2nmzA1", d.getDomainId());
213+
assertEquals(Boolean.TRUE, d.getManual(), "manual_rep is now True/False");
214+
assertEquals(Integer.valueOf(1), d.getXGroup());
215+
assertEquals(Integer.valueOf(1), d.getHGroup());
216+
assertEquals(Integer.valueOf(1), d.getTGroup());
217+
assertEquals(Integer.valueOf(3), d.getFGroup(), "f_id now carries a fourth level");
218+
assertEquals("2NMZ", d.getPdbId().getId());
219+
assertEquals("A", d.getChainId());
220+
assertEquals("A:1-99", d.getRange());
221+
assertEquals("A:1-99", d.getSeqIdRange());
222+
assertEquals("beta barrels", d.getArchitectureName());
223+
assertEquals("cradle loop barrel", d.getXGroupName());
224+
assertEquals("RIFT-related", d.getHGroupName());
225+
assertEquals("acid protease", d.getTGroupName());
226+
assertEquals("RVP", d.getFGroupName());
227+
// assembly_id is empty on every row of v294.1 and later, which means the same
228+
// as the NOT_DOMAIN_ASSEMBLY of earlier versions
229+
assertEquals(Long.valueOf(0), d.getAssemblyId());
230+
assertEquals(new LinkedHashSet<>(Arrays.asList("ROC", "SO4")), d.getLigands(),
231+
"the ligand list moved to ligand_comp_ids");
232+
}
233+
234+
/**
235+
* ligand_pdbnum, the last column, is empty on four rows in five. String.split
236+
* discards trailing empty fields unless asked not to, which would make those rows
237+
* look one column short.
238+
*/
239+
@Test
240+
void rowEndingInAnEmptyColumn() throws IOException {
241+
EcodDomain d = parse(V295).get(1);
242+
assertEquals("e2rspA1", d.getDomainId());
243+
assertEquals("2RSP", d.getPdbId().getId());
244+
assertEquals(Collections.emptySet(), d.getLigands());
245+
}
246+
247+
@Test
248+
void twentyThreeColumnsOfV2941() throws IOException {
249+
List<EcodDomain> domains = parse(String.join("\n",
250+
"# ECOD Domain List",
251+
"# Version: v294.1",
252+
"#",
253+
"uid\tecod_domain_id\tmanual_rep\tf_id\tpdb\tchain\tpdb_range\tseqid_range"
254+
+ "\tarchitecture_name\tx_name\th_name\tt_name\tf_name\tassembly_id"
255+
+ "\tdomain_id_short\trange_count\tarch_manual\tx_manual\th_manual"
256+
+ "\tt_manual\tf_manual\tvalid_structure\tligand_binding",
257+
"1\te1hvcA1\tFalse\t1.1.1.3\t1hvc\tA\tA:1B-99A\tA:1-203\tbeta barrels"
258+
+ "\tcradle loop barrel\tRIFT-related\tacid protease\tRVP\t\t\t\tFalse"
259+
+ "\tFalse\tFalse\tFalse\tFalse\tTrue\tFalse"));
260+
assertEquals(1, domains.size());
261+
EcodDomain d = domains.get(0);
262+
assertEquals("1HVC", d.getPdbId().getId());
263+
assertEquals("A:1B-99A", d.getRange());
264+
assertEquals("RVP", d.getFGroupName());
265+
// there is no ligand column at all in v294.1
266+
assertEquals(Collections.emptySet(), d.getLigands());
267+
}
268+
269+
@Test
270+
void columnHeaderIsNotADomain() throws IOException {
271+
// v294.1 stopped commenting the column names out, so they arrive looking like data
272+
for (EcodDomain d : parse(V295)) {
273+
assertFalse("uid".equals(d.getDomainId()));
274+
}
275+
}
276+
277+
/**
278+
* An empty f_name is not the same as F_UNCLASSIFIED: f_id still classifies the
279+
* domain to four levels, so the empty value is left as it is rather than translated.
280+
*/
281+
@Test
282+
void emptyFGroupNameIsLeftAlone() throws IOException {
283+
List<EcodDomain> domains = parse(String.join("\n",
284+
"# Version: v295",
285+
V295_COLUMNS,
286+
"7\te4fivA1\tTrue\t1.1.1.3\t4fiv\tA\tA:4-116\tA:1-113\tbeta barrels"
287+
+ "\tcradle loop barrel\tRIFT-related\tacid protease\t\t\t\t1\tFalse"
288+
+ "\tFalse\tFalse\tFalse\tTrue\tTrue\tTrue\tLP1\tA:201"));
289+
assertEquals(1, domains.size());
290+
assertEquals("", domains.get(0).getFGroupName());
291+
}
292+
}
293+
294+
@Nested
295+
class Robustness {
296+
@Test
297+
void unparseableLinesAreSkippedNotFatal() throws IOException {
298+
List<EcodDomain> domains = parse(String.join("\n",
299+
"# Version: v295",
300+
V295_COLUMNS,
301+
"not-a-number\tefoo\tTrue\t1.1.1.3\tfoo1\tA\tA:1-9\tA:1-9\ta\tb\tc\td\te"
302+
+ "\t\t\t1\tFalse\tFalse\tFalse\tFalse\tTrue\tTrue\tFalse\t\t",
303+
"0\te2nmzA1\tTrue\t1.1.1.3\t2nmz\tA\tA:1-99\tA:1-99\tbeta barrels"
304+
+ "\tcradle loop barrel\tRIFT-related\tacid protease\tRVP\t\t\t1"
305+
+ "\tFalse\tFalse\tFalse\tFalse\tTrue\tTrue\tTrue\tROC,SO4\tA:601"));
306+
assertEquals(1, domains.size(), "the good line is still read");
307+
assertEquals("e2nmzA1", domains.get(0).getDomainId());
308+
}
309+
310+
@Test
311+
void shortLineIsSkipped() throws IOException {
312+
assertTrue(parse(String.join("\n",
313+
"# Version: v295",
314+
V295_COLUMNS,
315+
"0\te2nmzA1\tTrue")).isEmpty());
316+
}
317+
318+
@Test
319+
void emptyFileYieldsNoDomains() throws IOException {
320+
assertTrue(parse("").isEmpty());
321+
}
322+
}
323+
}

0 commit comments

Comments
 (0)