commit 1b3fd15f92d5fe5f71e62d8a76ee847134cd5e07 Author: Karsten Loesing <karsten.loesing@gmx.net> Date: Mon Jun 26 22:12:10 2017 +0200 Tweak tests for removing deprecated code for #22154. This patch makes the following changes: - Remove now unused import in DescriptorParser. - Rename methods in DescriptorParserImpl to please checkstyle. - Tweak a few things in DescriptorReaderImplTest, including accepting varargs for dirs and putting back expected excluded files. - Remove tests that were previously testing deprecated methods which are now gone. - Change the two tests ending in NoDescriptors() back so that they really don't read any descriptors. - Put back tests for unrecognized lines, but don't expect DescriptorParseExceptions anymore and instead check getUnrecognizedLines(). --- .../torproject/descriptor/DescriptorParser.java | 1 - .../descriptor/impl/DescriptorParserImpl.java | 28 +++++++------- .../descriptor/impl/DescriptorParserImplTest.java | 5 +-- .../descriptor/impl/DescriptorReaderImplTest.java | 45 ++++++++-------------- .../impl/RelayNetworkStatusVoteImplTest.java | 42 +++++++++++++++++++- .../descriptor/impl/ServerDescriptorImplTest.java | 21 ++++++++++ 6 files changed, 93 insertions(+), 49 deletions(-) diff --git a/src/main/java/org/torproject/descriptor/DescriptorParser.java b/src/main/java/org/torproject/descriptor/DescriptorParser.java index 0ad2790..259510f 100644 --- a/src/main/java/org/torproject/descriptor/DescriptorParser.java +++ b/src/main/java/org/torproject/descriptor/DescriptorParser.java @@ -4,7 +4,6 @@ package org.torproject.descriptor; import java.io.File; -import java.util.List; /** * Descriptor source that parses descriptors from raw descriptor contents. diff --git a/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java b/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java index d39343d..ac7ff9c 100644 --- a/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java +++ b/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java @@ -52,7 +52,7 @@ public class DescriptorParserImpl implements DescriptorParser { NL + Key.NETWORK_STATUS_VERSION.keyword + SP + "3")) && firstLines.contains( NL + Key.VOTE_STATUS.keyword + SP + "consensus" + NL))) { - return this.parseDescriptors(rawDescriptorBytes, descriptorFile, + return this.parseOneOrMoreDescriptors(rawDescriptorBytes, descriptorFile, Key.NETWORK_STATUS_VERSION, RelayNetworkStatusConsensusImpl.class); } else if (firstLines.startsWith("@type network-status-vote-3 1.") || ((firstLines.startsWith( @@ -61,7 +61,7 @@ public class DescriptorParserImpl implements DescriptorParser { NL + Key.NETWORK_STATUS_VERSION.keyword + SP + "3" + NL)) && firstLines.contains( NL + Key.VOTE_STATUS.keyword + SP + "vote" + NL))) { - return this.parseDescriptors(rawDescriptorBytes, descriptorFile, + return this.parseOneOrMoreDescriptors(rawDescriptorBytes, descriptorFile, Key.NETWORK_STATUS_VERSION, RelayNetworkStatusVoteImpl.class); } else if (firstLines.startsWith("@type bridge-network-status 1.") || firstLines.startsWith(Key.R.keyword + SP)) { @@ -71,36 +71,36 @@ public class DescriptorParserImpl implements DescriptorParser { descriptorFile, fileName)); return parsedDescriptors; } else if (firstLines.startsWith("@type bridge-server-descriptor 1.")) { - return this.parseDescriptors(rawDescriptorBytes, descriptorFile, + return this.parseOneOrMoreDescriptors(rawDescriptorBytes, descriptorFile, Key.ROUTER, BridgeServerDescriptorImpl.class); } else if (firstLines.startsWith("@type server-descriptor 1.") || firstLines.startsWith(Key.ROUTER.keyword + SP) || firstLines.contains(NL + Key.ROUTER.keyword + SP)) { - return this.parseDescriptors(rawDescriptorBytes, descriptorFile, + return this.parseOneOrMoreDescriptors(rawDescriptorBytes, descriptorFile, Key.ROUTER, RelayServerDescriptorImpl.class); } else if (firstLines.startsWith("@type bridge-extra-info 1.")) { - return this.parseDescriptors(rawDescriptorBytes, descriptorFile, + return this.parseOneOrMoreDescriptors(rawDescriptorBytes, descriptorFile, Key.EXTRA_INFO, BridgeExtraInfoDescriptorImpl.class); } else if (firstLines.startsWith("@type extra-info 1.") || firstLines.startsWith(Key.EXTRA_INFO.keyword + SP) || firstLines.contains(NL + Key.EXTRA_INFO.keyword + SP)) { - return this.parseDescriptors(rawDescriptorBytes, descriptorFile, + return this.parseOneOrMoreDescriptors(rawDescriptorBytes, descriptorFile, Key.EXTRA_INFO, RelayExtraInfoDescriptorImpl.class); } else if (firstLines.startsWith("@type microdescriptor 1.") || firstLines.startsWith(Key.ONION_KEY.keyword + NL) || firstLines.contains(NL + Key.ONION_KEY.keyword + NL)) { - return this.parseDescriptors(rawDescriptorBytes, descriptorFile, + return this.parseOneOrMoreDescriptors(rawDescriptorBytes, descriptorFile, Key.ONION_KEY, MicrodescriptorImpl.class); } else if (firstLines.startsWith("@type bridge-pool-assignment 1.") || firstLines.startsWith(Key.BRIDGE_POOL_ASSIGNMENT.keyword + SP) || firstLines.contains(NL + Key.BRIDGE_POOL_ASSIGNMENT.keyword + SP)) { - return this.parseDescriptors(rawDescriptorBytes, descriptorFile, + return this.parseOneOrMoreDescriptors(rawDescriptorBytes, descriptorFile, Key.BRIDGE_POOL_ASSIGNMENT, BridgePoolAssignmentImpl.class); } else if (firstLines.startsWith("@type dir-key-certificate-3 1.") || firstLines.startsWith(Key.DIR_KEY_CERTIFICATE_VERSION.keyword + SP) || firstLines.contains( NL + Key.DIR_KEY_CERTIFICATE_VERSION.keyword + SP)) { - return this.parseDescriptors(rawDescriptorBytes, descriptorFile, + return this.parseOneOrMoreDescriptors(rawDescriptorBytes, descriptorFile, Key.DIR_KEY_CERTIFICATE_VERSION, DirectoryKeyCertificateImpl.class); } else if (firstLines.startsWith("@type tordnsel 1.") || firstLines.startsWith("ExitNode" + SP) @@ -114,12 +114,12 @@ public class DescriptorParserImpl implements DescriptorParser { Key.NETWORK_STATUS_VERSION.keyword + SP + "2" + NL) || firstLines.contains( NL + Key.NETWORK_STATUS_VERSION.keyword + SP + "2" + NL)) { - return this.parseDescriptors(rawDescriptorBytes, descriptorFile, + return this.parseOneOrMoreDescriptors(rawDescriptorBytes, descriptorFile, Key.NETWORK_STATUS_VERSION, RelayNetworkStatusImpl.class); } else if (firstLines.startsWith("@type directory 1.") || firstLines.startsWith(Key.SIGNED_DIRECTORY.keyword + NL) || firstLines.contains(NL + Key.SIGNED_DIRECTORY.keyword + NL)) { - return this.parseDescriptors(rawDescriptorBytes, descriptorFile, + return this.parseOneOrMoreDescriptors(rawDescriptorBytes, descriptorFile, Key.SIGNED_DIRECTORY, RelayDirectoryImpl.class); } else if (firstLines.startsWith("@type torperf 1.")) { return TorperfResultImpl.parseTorperfResults(rawDescriptorBytes, @@ -130,7 +130,7 @@ public class DescriptorParserImpl implements DescriptorParser { } } - private List<Descriptor> parseDescriptors(byte[] rawDescriptorBytes, + private List<Descriptor> parseOneOrMoreDescriptors(byte[] rawDescriptorBytes, File descriptorFile, Key key, Class<? extends DescriptorImpl> descriptorClass) throws DescriptorParseException { @@ -183,7 +183,7 @@ public class DescriptorParserImpl implements DescriptorParser { int[] offsetAndLength = new int[] { startAnnotations, endDescriptor - startAnnotations }; try { - parsedDescriptors.add(this.parseDescriptor(rawDescriptorBytes, + parsedDescriptors.add(this.parseOneDescriptor(rawDescriptorBytes, offsetAndLength, descriptorFile, constructor)); } catch (DescriptorParseException e) { parsedDescriptors.add(new UnparseableDescriptorImpl( @@ -194,7 +194,7 @@ public class DescriptorParserImpl implements DescriptorParser { return parsedDescriptors; } - Descriptor parseDescriptor(byte[] rawDescriptorBytes, + Descriptor parseOneDescriptor(byte[] rawDescriptorBytes, int[] offsetAndLength, File descriptorFile, Constructor<? extends DescriptorImpl> constructor) throws DescriptorParseException { diff --git a/src/test/java/org/torproject/descriptor/impl/DescriptorParserImplTest.java b/src/test/java/org/torproject/descriptor/impl/DescriptorParserImplTest.java index df441f3..42d0cde 100644 --- a/src/test/java/org/torproject/descriptor/impl/DescriptorParserImplTest.java +++ b/src/test/java/org/torproject/descriptor/impl/DescriptorParserImplTest.java @@ -20,8 +20,7 @@ public class DescriptorParserImplTest { @Rule public ExpectedException thrown = ExpectedException.none(); - private TestDescriptor makeTestDesc(byte[] bytes) - throws Exception { + private TestDescriptor makeTestDesc(byte[] bytes) throws Exception { return new TestDescriptor(bytes, new int[]{0, bytes.length}); } @@ -61,7 +60,7 @@ public class DescriptorParserImplTest { this.thrown.expectMessage("'176x.158.53.63' in line 'router UbuntuCore169 " + "176x.158.53.63 44583 0 0' is not a valid IPv4 address."); DescriptorParserImpl dpi = new DescriptorParserImpl(); - dpi.parseDescriptor(DEFECT.getBytes(), + dpi.parseOneDescriptor(DEFECT.getBytes(), new int[]{0, DEFECT.getBytes().length}, null, constructor); } diff --git a/src/test/java/org/torproject/descriptor/impl/DescriptorReaderImplTest.java b/src/test/java/org/torproject/descriptor/impl/DescriptorReaderImplTest.java index a00e5ba..0a3791e 100644 --- a/src/test/java/org/torproject/descriptor/impl/DescriptorReaderImplTest.java +++ b/src/test/java/org/torproject/descriptor/impl/DescriptorReaderImplTest.java @@ -63,9 +63,9 @@ public class DescriptorReaderImplTest { parseHistoryContents.getBytes(StandardCharsets.UTF_8)); } - private int readAllDescriptors(File dir) { + private int readAllDescriptors(File... dirs) { Iterator<Descriptor> descriptors = this.descriptorReader - .readDescriptors(dir).iterator(); + .readDescriptors(dirs).iterator(); int count = 0; while (descriptors.hasNext()) { count++; @@ -75,8 +75,10 @@ public class DescriptorReaderImplTest { } private void assertExcludedFilesParsedFilesAndHistoryFileLines( - int expectedParsedFiles, + int expectedExcludedFiles, int expectedParsedFiles, int expectedHistoryFileLines) throws IOException { + assertEquals(expectedExcludedFiles, + this.descriptorReader.getExcludedFiles().size()); assertEquals(expectedParsedFiles, this.descriptorReader.getParsedFiles().size()); assertEquals(expectedHistoryFileLines, @@ -87,38 +89,21 @@ public class DescriptorReaderImplTest { @Test public void testDescriptors() throws IOException { this.readAllDescriptors(this.inputDirectory); - this.assertExcludedFilesParsedFilesAndHistoryFileLines(2, 1); + this.assertExcludedFilesParsedFilesAndHistoryFileLines(0, 2, 1); } @Test public void testNoDescriptors() throws Exception { // calling readAllDescriptors fails with IAE Iterator<Descriptor> descriptors = this.descriptorReader - .readDescriptors(null).iterator(); + .readDescriptors().iterator(); int count = 0; while (descriptors.hasNext()) { count++; descriptors.next(); } assertEquals(count, 0); - this.assertExcludedFilesParsedFilesAndHistoryFileLines(0, 1); - } - - @Test - @SuppressWarnings("deprecation") - public void testSetExcludeFilesDescriptors() throws InterruptedException, - IOException { - this.readAllDescriptors(this.inputDirectory); - Thread.sleep(100L); /* It may take a moment to write the history file. */ - this.assertExcludedFilesParsedFilesAndHistoryFileLines(2, 1); - } - - @Test - public void testSetExcludeFilesNoDescriptors() throws InterruptedException, - IOException { - this.readAllDescriptors(this.inputDirectory); - Thread.sleep(100L); /* It may take a moment to write the history file. */ - this.assertExcludedFilesParsedFilesAndHistoryFileLines(2, 1); + this.assertExcludedFilesParsedFilesAndHistoryFileLines(0, 0, 1); } @Test @@ -126,29 +111,29 @@ public class DescriptorReaderImplTest { this.descriptorReader.setHistoryFile(this.historyFile); this.readAllDescriptors(this.inputDirectory); descriptorReader.saveHistoryFile(this.historyFile); - this.assertExcludedFilesParsedFilesAndHistoryFileLines(1, 2); + this.assertExcludedFilesParsedFilesAndHistoryFileLines(1, 1, 2); } @Test public void testSetHistoryFileNoDescriptors() throws IOException { this.descriptorReader.setHistoryFile(this.historyFile); - this.readAllDescriptors(this.inputDirectory); + this.readAllDescriptors(); this.descriptorReader.saveHistoryFile(this.historyFile); - this.assertExcludedFilesParsedFilesAndHistoryFileLines(1, 2); + this.assertExcludedFilesParsedFilesAndHistoryFileLines(0, 0, 0); } @Test public void testSetExcludedFilesDescriptors() throws IOException { this.descriptorReader.setExcludedFiles(this.historyMap); this.readAllDescriptors(this.inputDirectory); - this.assertExcludedFilesParsedFilesAndHistoryFileLines(1, 1); + this.assertExcludedFilesParsedFilesAndHistoryFileLines(1, 1, 1); } @Test public void testSetExcludedFilesNoDescriptors() throws IOException { this.descriptorReader.setExcludedFiles(this.historyMap); - this.readAllDescriptors(this.inputDirectory); - this.assertExcludedFilesParsedFilesAndHistoryFileLines(1, 1); + this.readAllDescriptors(); + this.assertExcludedFilesParsedFilesAndHistoryFileLines(0, 0, 1); } @Test @@ -159,7 +144,7 @@ public class DescriptorReaderImplTest { int count = this.readAllDescriptors(this.inputDirectory); assertEquals("Two files should have been parsed.", 2, count); descriptorReader.saveHistoryFile(this.historyFile); - this.assertExcludedFilesParsedFilesAndHistoryFileLines(2, 2); + this.assertExcludedFilesParsedFilesAndHistoryFileLines(0, 2, 2); } } diff --git a/src/test/java/org/torproject/descriptor/impl/RelayNetworkStatusVoteImplTest.java b/src/test/java/org/torproject/descriptor/impl/RelayNetworkStatusVoteImplTest.java index 8dc89bd..aba55f4 100644 --- a/src/test/java/org/torproject/descriptor/impl/RelayNetworkStatusVoteImplTest.java +++ b/src/test/java/org/torproject/descriptor/impl/RelayNetworkStatusVoteImplTest.java @@ -962,6 +962,16 @@ public class RelayNetworkStatusVoteImplTest { } @Test + public void testConsensusMethodTwoLines() + throws DescriptorParseException { + String consensusMethodLine = "consensus-method 1"; + RelayNetworkStatusVote vote = VoteBuilder.createWithConsensusMethodsLine( + consensusMethodLine + "\n" + consensusMethodLine); + assertEquals(Arrays.asList(consensusMethodLine, consensusMethodLine), + vote.getUnrecognizedLines()); + } + + @Test public void testPublishedNoLine() throws DescriptorParseException { this.thrown.expect(DescriptorParseException.class); this.thrown.expectMessage("Keyword 'published' is contained 0 times, " @@ -1770,7 +1780,7 @@ public class RelayNetworkStatusVoteImplTest { assertEquals(signingKeyDigestSha1, secondSignature.getSigningKeyDigestSha1Hex()); assertEquals(signatureSha1 + "\n", secondSignature.getSignature()); - assertEquals(signingKeyDigestSha1, vote.getSignatures().get(0) + assertEquals(signingKeyDigestSha1, vote.getSignatures().get(1) .getSigningKeyDigestSha1Hex()); assertEquals("c0d58c8d3c3695526f6eb5c0d9f8452b2234d303", vote.getDigestSha1Hex()); @@ -1789,6 +1799,16 @@ public class RelayNetworkStatusVoteImplTest { } @Test + public void testUnrecognizedHeaderLineFail() + throws DescriptorParseException { + String unrecognizedLine = "unrecognized-line 1"; + RelayNetworkStatusVote vote = VoteBuilder.createWithUnrecognizedHeaderLine( + unrecognizedLine); + assertEquals(Arrays.asList(unrecognizedLine), + vote.getUnrecognizedLines()); + } + + @Test public void testUnrecognizedHeaderLineIgnore() throws DescriptorParseException { String unrecognizedLine = "unrecognized-line 1"; @@ -1800,6 +1820,16 @@ public class RelayNetworkStatusVoteImplTest { } @Test + public void testUnrecognizedDirSourceLineFail() + throws DescriptorParseException { + String unrecognizedLine = "unrecognized-line 1"; + RelayNetworkStatusVote vote = + VoteBuilder.createWithUnrecognizedDirSourceLine(unrecognizedLine); + assertEquals(Arrays.asList(unrecognizedLine), + vote.getUnrecognizedLines()); + } + + @Test public void testUnrecognizedDirSourceLineIgnore() throws DescriptorParseException { String unrecognizedLine = "unrecognized-line 1"; @@ -1811,6 +1841,16 @@ public class RelayNetworkStatusVoteImplTest { } @Test + public void testUnrecognizedFooterLineFail() + throws DescriptorParseException { + String unrecognizedLine = "unrecognized-line 1"; + RelayNetworkStatusVote vote = VoteBuilder.createWithUnrecognizedFooterLine( + unrecognizedLine); + assertEquals(Arrays.asList(unrecognizedLine), + vote.getUnrecognizedLines()); + } + + @Test public void testUnrecognizedFooterLineIgnore() throws DescriptorParseException { String unrecognizedLine = "unrecognized-line 1"; diff --git a/src/test/java/org/torproject/descriptor/impl/ServerDescriptorImplTest.java b/src/test/java/org/torproject/descriptor/impl/ServerDescriptorImplTest.java index 972e207..534cb0e 100644 --- a/src/test/java/org/torproject/descriptor/impl/ServerDescriptorImplTest.java +++ b/src/test/java/org/torproject/descriptor/impl/ServerDescriptorImplTest.java @@ -1675,6 +1675,16 @@ public class ServerDescriptorImplTest { } @Test + public void testTunnelledDirServerTypo() + throws DescriptorParseException { + String tunneledDirServerLine = "tunneled-dir-server"; + ServerDescriptor descriptor = DescriptorBuilder + .createWithTunnelledDirServerLine(tunneledDirServerLine); + assertEquals(Arrays.asList(tunneledDirServerLine), + descriptor.getUnrecognizedLines()); + } + + @Test public void testTunnelledDirServerTwice() throws DescriptorParseException { this.thrown.expect(DescriptorParseException.class); @@ -1858,6 +1868,17 @@ public class ServerDescriptorImplTest { + ROUTER_SIG_ED25519_LINE); } + @Test + public void testEd25519FollowedbyUnrecognizedLine() + throws DescriptorParseException { + String unrecognizedLine = "unrecognized-line 1"; + ServerDescriptor serverDecriptor = DescriptorBuilder.createWithEd25519Lines( + IDENTITY_ED25519_LINES, MASTER_KEY_ED25519_LINE, ROUTER_SIG_ED25519_LINE + + "\n" + unrecognizedLine); + assertEquals(Arrays.asList(unrecognizedLine), + serverDecriptor.getUnrecognizedLines()); + } + private static final String ONION_KEY_CROSSCERT_LINES = "onion-key-crosscert\n" + "-----BEGIN CROSSCERT-----\n"