From 2b19129bb95665a31d5a4657972115324952e95c Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Tue, 16 Oct 2018 08:32:50 +1000 Subject: [PATCH] [NC-1615] Report reference tests that have been blacklisted rather than skipping them silently (#63) --- .../pantheon/ethereum/core/TransactionTest.java | 8 ++++++-- .../ethereum/vm/BlockchainReferenceTestTools.java | 4 +--- .../ethereum/vm/GeneralStateReferenceTestTools.java | 8 +++----- .../pantheon/ethereum/vm/VMReferenceTest.java | 5 ++++- .../vm/BlockchainReferenceTest.java.template | 6 +++++- .../vm/GeneralStateReferenceTest.java.template | 6 +++++- .../pantheon/ethereum/rlp/InvalidRLPRefTest.java | 6 +++++- .../pegasys/pantheon/ethereum/rlp/RLPRefTest.java | 4 +++- .../pegasys/pantheon/ethereum/trie/TrieRefTest.java | 4 +++- .../pantheon/testutil/JsonTestParameters.java | 12 +++++------- 10 files changed, 40 insertions(+), 23 deletions(-) diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java index 2994b81cd4..4c7b1601bd 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java @@ -1,5 +1,7 @@ package tech.pegasys.pantheon.ethereum.core; +import static org.junit.Assume.assumeTrue; + import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator; import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.ethereum.vm.ReferenceTestProtocolSchedules; @@ -39,12 +41,14 @@ public class TransactionTest { "TransactionWithGasLimitOverflow(2|63)", "TransactionWithGasLimitxPriceOverflow$") // Nonce is tracked with type long, large valued nonces can't currently be decoded .blacklist("TransactionWithHighNonce256") - .generator((name, spec, collector) -> collector.add(name, spec)) + .generator((name, spec, collector) -> collector.add(name, spec, true)) .generate(TEST_CONFIG_FILE_DIR_PATH); } - public TransactionTest(final String name, final TransactionTestCaseSpec spec) { + public TransactionTest( + final String name, final TransactionTestCaseSpec spec, final boolean runTest) { this.spec = spec; + assumeTrue("Test was blacklisted", runTest); } @Test diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTestTools.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTestTools.java index c592b3cd9f..fed258fc26 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTestTools.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTestTools.java @@ -40,9 +40,7 @@ public class BlockchainReferenceTestTools { .generator( (testName, spec, collector) -> { final String eip = spec.getNetwork(); - if (NETWORKS_TO_RUN.contains(eip)) { - collector.add(testName + "[" + eip + "]", spec); - } + collector.add(testName + "[" + eip + "]", spec, NETWORKS_TO_RUN.contains(eip)); }); static { diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java index bd2aae2dd3..427e3db3c4 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java @@ -47,15 +47,13 @@ public class GeneralStateReferenceTestTools { for (final Map.Entry> entry : stateSpec.finalStateSpecs().entrySet()) { final String eip = entry.getKey(); - if (!EIPS_TO_RUN.contains(eip)) { - continue; - } + final boolean runTest = EIPS_TO_RUN.contains(eip); final List eipSpecs = entry.getValue(); if (eipSpecs.size() == 1) { - collector.add(prefix + eip, eipSpecs.get(0)); + collector.add(prefix + eip, eipSpecs.get(0), runTest); } else { for (int i = 0; i < eipSpecs.size(); i++) { - collector.add(prefix + eip + '[' + i + ']', eipSpecs.get(i)); + collector.add(prefix + eip + '[' + i + ']', eipSpecs.get(i), runTest); } } } diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/VMReferenceTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/VMReferenceTest.java index 06d2df9a64..fc9b804758 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/VMReferenceTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/VMReferenceTest.java @@ -3,6 +3,7 @@ package tech.pegasys.pantheon.ethereum.vm; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; import static tech.pegasys.pantheon.ethereum.vm.OperationTracer.NO_TRACING; import tech.pegasys.pantheon.ethereum.core.Gas; @@ -88,9 +89,11 @@ public class VMReferenceTest extends AbstractRetryingTest { .generate(TEST_CONFIG_FILE_DIR_PATHS); } - public VMReferenceTest(final String name, final VMReferenceTestCaseSpec spec) { + public VMReferenceTest( + final String name, final VMReferenceTestCaseSpec spec, final boolean runTest) { this.name = name; this.spec = spec; + assumeTrue("Test was blacklisted", runTest); } @Override diff --git a/ethereum/core/src/test/resources/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTest.java.template b/ethereum/core/src/test/resources/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTest.java.template index 00113b4803..4a817d0d73 100644 --- a/ethereum/core/src/test/resources/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTest.java.template +++ b/ethereum/core/src/test/resources/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTest.java.template @@ -12,6 +12,8 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; +import static org.junit.Assume.assumeTrue; + /** The blockchain test operation testing framework entry point. */ @RunWith(Parameterized.class) public class %%TESTS_NAME%% { @@ -28,9 +30,11 @@ public class %%TESTS_NAME%% { public %%TESTS_NAME%%( final String name, - final BlockchainReferenceTestCaseSpec spec) { + final BlockchainReferenceTestCaseSpec spec, + final boolean runTest) { this.name = name; this.spec = spec; + assumeTrue("Test was blacklisted", runTest); } @Test diff --git a/ethereum/core/src/test/resources/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTest.java.template b/ethereum/core/src/test/resources/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTest.java.template index 5896b09461..8a1c12ba11 100644 --- a/ethereum/core/src/test/resources/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTest.java.template +++ b/ethereum/core/src/test/resources/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTest.java.template @@ -12,6 +12,8 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; +import static org.junit.Assume.assumeTrue; + /** The general state test operation testing framework entry point. */ @RunWith(Parameterized.class) public class %%TESTS_NAME%% { @@ -28,9 +30,11 @@ public class %%TESTS_NAME%% { public %%TESTS_NAME%%( final String name, - final GeneralStateTestCaseEipSpec spec) { + final GeneralStateTestCaseEipSpec spec, + final boolean runTest) { this.name = name; this.spec = spec; + assumeTrue("Test was blacklisted", runTest); } @Test diff --git a/ethereum/rlp/src/test/java/tech/pegasys/pantheon/ethereum/rlp/InvalidRLPRefTest.java b/ethereum/rlp/src/test/java/tech/pegasys/pantheon/ethereum/rlp/InvalidRLPRefTest.java index 1b58b28fde..8ce522859c 100644 --- a/ethereum/rlp/src/test/java/tech/pegasys/pantheon/ethereum/rlp/InvalidRLPRefTest.java +++ b/ethereum/rlp/src/test/java/tech/pegasys/pantheon/ethereum/rlp/InvalidRLPRefTest.java @@ -1,5 +1,7 @@ package tech.pegasys.pantheon.ethereum.rlp; +import static org.junit.Assume.assumeTrue; + import tech.pegasys.pantheon.testutil.JsonTestParameters; import java.util.Collection; @@ -20,8 +22,10 @@ public class InvalidRLPRefTest { private final InvalidRLPRefTestCaseSpec spec; - public InvalidRLPRefTest(final String name, final InvalidRLPRefTestCaseSpec spec) { + public InvalidRLPRefTest( + final String name, final InvalidRLPRefTestCaseSpec spec, final boolean runTest) { this.spec = spec; + assumeTrue("Test was blacklisted", runTest); } @Parameters(name = "Name: {0}") diff --git a/ethereum/rlp/src/test/java/tech/pegasys/pantheon/ethereum/rlp/RLPRefTest.java b/ethereum/rlp/src/test/java/tech/pegasys/pantheon/ethereum/rlp/RLPRefTest.java index a422e0330f..7678ca9066 100644 --- a/ethereum/rlp/src/test/java/tech/pegasys/pantheon/ethereum/rlp/RLPRefTest.java +++ b/ethereum/rlp/src/test/java/tech/pegasys/pantheon/ethereum/rlp/RLPRefTest.java @@ -1,6 +1,7 @@ package tech.pegasys.pantheon.ethereum.rlp; import static org.junit.Assert.assertEquals; +import static org.junit.Assume.assumeTrue; import tech.pegasys.pantheon.testutil.JsonTestParameters; @@ -19,8 +20,9 @@ public class RLPRefTest { private final RLPRefTestCaseSpec spec; - public RLPRefTest(final String name, final RLPRefTestCaseSpec spec) { + public RLPRefTest(final String name, final RLPRefTestCaseSpec spec, final boolean runTest) { this.spec = spec; + assumeTrue("Test was blacklisted", runTest); } @Parameters(name = "Name: {0}") diff --git a/ethereum/trie/src/test/java/tech/pegasys/pantheon/ethereum/trie/TrieRefTest.java b/ethereum/trie/src/test/java/tech/pegasys/pantheon/ethereum/trie/TrieRefTest.java index 54167e2274..28fa5eee7c 100644 --- a/ethereum/trie/src/test/java/tech/pegasys/pantheon/ethereum/trie/TrieRefTest.java +++ b/ethereum/trie/src/test/java/tech/pegasys/pantheon/ethereum/trie/TrieRefTest.java @@ -1,6 +1,7 @@ package tech.pegasys.pantheon.ethereum.trie; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assume.assumeTrue; import tech.pegasys.pantheon.testutil.JsonTestParameters; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -20,8 +21,9 @@ public class TrieRefTest { private final TrieRefTestCaseSpec spec; - public TrieRefTest(final String name, final TrieRefTestCaseSpec spec) { + public TrieRefTest(final String name, final TrieRefTestCaseSpec spec, final boolean runTest) { this.spec = spec; + assumeTrue("Test was blacklisted", runTest); } @Parameters(name = "Name: {0}") diff --git a/testutil/src/main/java/tech/pegasys/pantheon/testutil/JsonTestParameters.java b/testutil/src/main/java/tech/pegasys/pantheon/testutil/JsonTestParameters.java index 44eee5291e..93b7e8fef1 100644 --- a/testutil/src/main/java/tech/pegasys/pantheon/testutil/JsonTestParameters.java +++ b/testutil/src/main/java/tech/pegasys/pantheon/testutil/JsonTestParameters.java @@ -50,10 +50,8 @@ public class JsonTestParameters { // memory when we run a single test, but it's not the case we're trying to optimize. private final List testParameters = new ArrayList<>(256); - public void add(final String name, final S value) { - if (includes(name)) { - testParameters.add(new Object[] {name, value}); - } + public void add(final String name, final S value, final boolean runTest) { + testParameters.add(new Object[] {name, value, runTest && includes(name)}); } private boolean includes(final String name) { @@ -104,7 +102,7 @@ public class JsonTestParameters { public static JsonTestParameters create(final Class testCaseSpec) { return new JsonTestParameters<>(testCaseSpec, testCaseSpec) - .generator((name, testCase, collector) -> collector.add(name, testCase)); + .generator((name, testCase, collector) -> collector.add(name, testCase, true)); } public static JsonTestParameters create( @@ -187,13 +185,13 @@ public class JsonTestParameters { for (final String path : paths) { final URL url = classLoader.getResource(path); checkState(url != null, "Cannot find test directory " + path); - Path dir; + final Path dir; try { dir = Paths.get(url.toURI()); } catch (final URISyntaxException e) { throw new RuntimeException("Problem converting URL to URI " + url, e); } - try (Stream s = Files.walk(dir)) { + try (final Stream s = Files.walk(dir)) { s.map(Path::toFile) .filter(f -> f.getPath().endsWith(".json")) .filter(f -> !fileExcludes.contains(f.getName()))