From 4c1edc9c3cec61127f45074abaf406573a014292 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Fri, 28 Aug 2026 20:52:23 +0200 Subject: [PATCH] test: Collect the test files in one place Both drivers walked the directory themselves, with the same recursive search, the same index.json skip, the same sort and the same suite naming written out twice. Only the registration at the end differed. collect_test_files() in testutils does the walk once and returns each file with the suite it belongs to. The listed tests are unchanged, name for name: 20245 for the state tool and 21333 for the blockchain one, in the same order. The integration tests grow to cover the paths they were missing. The blockchain tool was only ever given single files, so the whole directory form went unrun; SuiteA gains files that must not be collected; and a fixture with an unsupported block shape exercises both places that shape is reported. Coverage of blockchaintest.cpp goes from 61% to 97% of lines, and of the collector to 100%. --- test/blockchaintest/blockchaintest.cpp | 14 ++---- .../integration/blockchaintest/CMakeLists.txt | 26 +++++++++- .../blockchaintest/unsupported_rlp.json | 47 +++++++++++++++++++ test/integration/statetest/CMakeLists.txt | 15 ++++++ .../statetest/tests1/SuiteA/index.json | 4 ++ .../statetest/tests1/SuiteA/notes.txt | 1 + test/statetest/statetest.cpp | 15 ++---- test/utils/CMakeLists.txt | 2 + test/utils/test_files.cpp | 34 ++++++++++++++ test/utils/test_files.hpp | 28 +++++++++++ 10 files changed, 161 insertions(+), 25 deletions(-) create mode 100644 test/integration/blockchaintest/unsupported_rlp.json create mode 100644 test/integration/statetest/tests1/SuiteA/index.json create mode 100644 test/integration/statetest/tests1/SuiteA/notes.txt create mode 100644 test/utils/test_files.cpp create mode 100644 test/utils/test_files.hpp diff --git a/test/blockchaintest/blockchaintest.cpp b/test/blockchaintest/blockchaintest.cpp index 635a4d167d..f816ec532b 100644 --- a/test/blockchaintest/blockchaintest.cpp +++ b/test/blockchaintest/blockchaintest.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -91,17 +92,8 @@ void register_test_files(const fs::path& root, evmc::VM& vm) { if (is_directory(root)) { - std::vector test_files; - std::copy_if(fs::recursive_directory_iterator{root}, fs::recursive_directory_iterator{}, - std::back_inserter(test_files), [](const fs::directory_entry& entry) { - // "index.json" files are just lists of tests generated by other tools. - return entry.is_regular_file() && entry.path().extension() == ".json" && - entry.path().filename() != "index.json"; - }); - std::ranges::sort(test_files); - - for (const auto& p : test_files) - BlockchainGTestFile::register_one(fs::relative(p, root).parent_path().string(), p, vm); + for (const auto& [path, suite_name] : evmone::test::collect_test_files(root)) + BlockchainGTestFile::register_one(suite_name, path, vm); } else // Treat as a file. { diff --git a/test/integration/blockchaintest/CMakeLists.txt b/test/integration/blockchaintest/CMakeLists.txt index f15ee5363c..ade6350d7a 100644 --- a/test/integration/blockchaintest/CMakeLists.txt +++ b/test/integration/blockchaintest/CMakeLists.txt @@ -17,10 +17,10 @@ set_tests_properties( PASS_REGULAR_EXPRESSION ".*2 tests from" ) -# Exercise block-level gas accounting (EIP-7778). +# Exercise block-level gas accounting (EIP-7778), with tracing on so that flag is covered too. add_test( NAME ${PREFIX}/eip7778_block_gas - COMMAND evmone-blockchaintest ${TESTS1}/eip7778_block_gas.json + COMMAND evmone-blockchaintest ${TESTS1}/eip7778_block_gas.json --trace ) # A block invalid by its signature must not be judged by another check. @@ -33,5 +33,27 @@ add_test( COMMAND evmone-blockchaintest ${TESTS1}/unrecovered_sender_deposit_log.json ) +# A block with expectException but no rlp_decoded is a shape the loader does not support. As a +# single file it is reported and nothing is registered. +add_test( + NAME ${PREFIX}/unsupported_rlp + COMMAND evmone-blockchaintest ${TESTS1}/unsupported_rlp.json +) +set_tests_properties( + ${PREFIX}/unsupported_rlp PROPERTIES + PASS_REGULAR_EXPRESSION "tests with invalidly rlp-encoded blocks are not supported" +) + +# The directory form registers one test per file, which none of the tests above reach, and skips +# the unsupported one rather than failing on it. +add_test( + NAME ${PREFIX}/directory + COMMAND evmone-blockchaintest ${TESTS1} +) +set_tests_properties( + ${PREFIX}/directory PROPERTIES + PASS_REGULAR_EXPRESSION "SKIPPED \\] \\.unsupported_rlp" +) + get_directory_property(ALL_TESTS TESTS) set_tests_properties(${ALL_TESTS} PROPERTIES ENVIRONMENT LLVM_PROFILE_FILE=${CMAKE_BINARY_DIR}/integration-%p.profraw) diff --git a/test/integration/blockchaintest/unsupported_rlp.json b/test/integration/blockchaintest/unsupported_rlp.json new file mode 100644 index 0000000000..78540fd57c --- /dev/null +++ b/test/integration/blockchaintest/unsupported_rlp.json @@ -0,0 +1,47 @@ +{ + "unsupported_rlp": { + "_info": { + "comment": "The runner takes the sender from the fixture, so it cannot see the broken signature; the transaction executes and the wrong blobGasUsed is the first header mismatch." + }, + "network": "Cancun", + "genesisBlockHeader": { + "parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000", + "coinbase": "0x0000000000000000000000000000000000000000", + "stateRoot": "0x0000000000000000000000000000000000000000000000000000000000000000", + "transactionsTrie": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421", + "receiptTrie": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421", + "bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "number": "0x00", + "gasLimit": "0x01000000", + "gasUsed": "0x00", + "timestamp": "0x00", + "extraData": "0x00", + "baseFeePerGas": "0x10", + "withdrawalsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421", + "blobGasUsed": "0x00", + "excessBlobGas": "0x00", + "hash": "0x0000000000000000000000000000000000000000000000000000000000000001" + }, + "pre": { + "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b": { + "code": "0x", + "nonce": "0x00", + "balance": "0x02540be400" + } + }, + "blocks": [ + { + "expectException": "TransactionException.INVALID_SIGNATURE_VRS", + "rlp": "0x" + } + ], + "lastblockhash": "0x0000000000000000000000000000000000000000000000000000000000000001", + "postState": { + "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b": { + "code": "0x", + "nonce": "0x00", + "balance": "0x02540be400" + } + } + } +} diff --git a/test/integration/statetest/CMakeLists.txt b/test/integration/statetest/CMakeLists.txt index e21cfd23eb..224fe2589f 100644 --- a/test/integration/statetest/CMakeLists.txt +++ b/test/integration/statetest/CMakeLists.txt @@ -19,6 +19,8 @@ set_tests_properties( PASS_REGULAR_EXPRESSION "path is required" ) +# SuiteA also holds an index.json and a notes.txt, neither of which is a test. They sort before +# test1.json, so collecting either would show up inside the SuiteA block below. add_test( NAME ${PREFIX}/tests1_list COMMAND evmone-statetest ${TESTS1} --gtest_list_tests @@ -134,5 +136,18 @@ set_tests_properties( FAIL_REGULAR_EXPRESSION "failing_test_case" ) +# Over a directory the filter is applied per case inside the file's test, not at registration as +# it is above. The summary line is what proves a case ran: forbidding the other name alone would +# hold just as well if the filter dropped every case. +add_test( + NAME ${PREFIX}/filter_directory + COMMAND evmone-statetest ${TESTS_FILTER} -k passing_test_case --trace-summary +) +set_tests_properties( + ${PREFIX}/filter_directory PROPERTIES + PASS_REGULAR_EXPRESSION "\"pass\":true" + FAIL_REGULAR_EXPRESSION "failing_test_case" +) + get_directory_property(ALL_TESTS TESTS) set_tests_properties(${ALL_TESTS} PROPERTIES ENVIRONMENT LLVM_PROFILE_FILE=${CMAKE_BINARY_DIR}/integration-%p.profraw) diff --git a/test/integration/statetest/tests1/SuiteA/index.json b/test/integration/statetest/tests1/SuiteA/index.json new file mode 100644 index 0000000000..da71b73f4a --- /dev/null +++ b/test/integration/statetest/tests1/SuiteA/index.json @@ -0,0 +1,4 @@ +{ + "comment": "A listing generated by other tools, not a test. It must not be collected.", + "tests": ["test1.json", "test2_multi.json"] +} diff --git a/test/integration/statetest/tests1/SuiteA/notes.txt b/test/integration/statetest/tests1/SuiteA/notes.txt new file mode 100644 index 0000000000..2f579bf55e --- /dev/null +++ b/test/integration/statetest/tests1/SuiteA/notes.txt @@ -0,0 +1 @@ +Not a test either: only .json files are collected. diff --git a/test/statetest/statetest.cpp b/test/statetest/statetest.cpp index e68e0f9690..f1b0594a98 100644 --- a/test/statetest/statetest.cpp +++ b/test/statetest/statetest.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -96,18 +97,8 @@ void register_test_files( { if (is_directory(root)) { - std::vector test_files; - std::copy_if(fs::recursive_directory_iterator{root}, fs::recursive_directory_iterator{}, - std::back_inserter(test_files), [](const fs::directory_entry& entry) { - // "index.json" files are just lists of tests generated by other tools. - return entry.is_regular_file() && entry.path().extension() == ".json" && - entry.path().filename() != "index.json"; - }); - std::ranges::sort(test_files); - - for (const auto& p : test_files) - StateTestFile::register_one( - fs::relative(p, root).parent_path().string(), p, filter, vm, trace); + for (const auto& [path, suite_name] : evmone::test::collect_test_files(root)) + StateTestFile::register_one(suite_name, path, filter, vm, trace); } else // Treat as a file. { diff --git a/test/utils/CMakeLists.txt b/test/utils/CMakeLists.txt index 8b1d414ff4..88292d9b44 100644 --- a/test/utils/CMakeLists.txt +++ b/test/utils/CMakeLists.txt @@ -40,6 +40,8 @@ target_sources( statetest_runner.cpp t8n.hpp t8n.cpp + test_files.hpp + test_files.cpp test_report.hpp test_report.cpp test_state.hpp diff --git a/test/utils/test_files.cpp b/test/utils/test_files.cpp new file mode 100644 index 0000000000..ce1db39057 --- /dev/null +++ b/test/utils/test_files.cpp @@ -0,0 +1,34 @@ +// evmone: Fast Ethereum Virtual Machine implementation +// Copyright 2026 The evmone Authors. +// SPDX-License-Identifier: Apache-2.0 + +#include "test_files.hpp" +#include +#include + +namespace evmone::test +{ +namespace fs = std::filesystem; + +std::vector collect_test_files(const fs::path& root) +{ + static constexpr auto is_test_file = [](const fs::directory_entry& entry) { + return entry.is_regular_file() && entry.path().extension() == ".json" && + entry.path().filename() != "index.json"; + }; + const auto as_test_file = [&root](const fs::directory_entry& entry) { + return TestFile{entry.path(), fs::relative(entry.path(), root).parent_path().string()}; + }; + // TODO(gcc-12): Pipe the temporary in directly. Adapting one needs owning_view, which C++20 + // has but gcc-11's libstdc++ does not implement. + const fs::recursive_directory_iterator entries{root}; + + // TODO(C++23): std::ranges::to() replaces the vector and the copy. + std::vector files; + std::ranges::copy( + entries | std::views::filter(is_test_file) | std::views::transform(as_test_file), + std::back_inserter(files)); + std::ranges::sort(files); + return files; +} +} // namespace evmone::test diff --git a/test/utils/test_files.hpp b/test/utils/test_files.hpp new file mode 100644 index 0000000000..6f07e0c1ed --- /dev/null +++ b/test/utils/test_files.hpp @@ -0,0 +1,28 @@ +// evmone: Fast Ethereum Virtual Machine implementation +// Copyright 2026 The evmone Authors. +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include +#include +#include + +namespace evmone::test +{ +/// A test file found under a search root. +struct TestFile +{ + std::filesystem::path path; + + /// The file's directory relative to the root, which is the test suite it belongs to. + std::string suite_name; + + /// Ordered by path, because that is the order a run visits them in. The suite name is derived + /// from the path, so it never gets to decide a comparison. + auto operator<=>(const TestFile&) const = default; +}; + +/// The JSON test files under @p root, sorted, so a run visits them in a stable order. +/// "index.json" is skipped: those are lists of tests generated by other tools, not tests. +[[nodiscard]] std::vector collect_test_files(const std::filesystem::path& root); +} // namespace evmone::test