Merge pull request #12436 from ethereum/testBatcher

Test batcher.

.circleci/config.yml

@@ -897,7 +897,7 @@ jobs:
 
   t_ubu_soltest_all: &t_ubu_soltest_all
     <<: *base_ubuntu2004
-    parallelism: 15 # 7 EVM versions, each with/without optimization + 1 ABIv1/@nooptions run
+    parallelism: 50
     <<: *steps_soltest_all
 
   t_ubu_lsp: &t_ubu_lsp
@@ -906,6 +906,7 @@ jobs:
 
   t_archlinux_soltest: &t_archlinux_soltest
     <<: *base_archlinux
+    parallelism: 20
     environment:
       EVM: << pipeline.parameters.evm-version >>
       OPTIMIZE: 0
@@ -924,6 +925,7 @@ jobs:
 
   t_ubu_soltest_enforce_yul: &t_ubu_soltest_enforce_yul
     <<: *base_ubuntu2004
+    parallelism: 20
     environment:
       EVM: << pipeline.parameters.evm-version >>
       SOLTEST_FLAGS: --enforce-via-yul
@@ -933,6 +935,7 @@ jobs:
 
   t_ubu_clang_soltest: &t_ubu_clang_soltest
     <<: *base_ubuntu2004_clang
+    parallelism: 20
     environment:
       EVM: << pipeline.parameters.evm-version >>
       OPTIMIZE: 0
@@ -960,6 +963,7 @@ jobs:
 
   t_ubu_asan_soltest:
     <<: *base_ubuntu2004
+    parallelism: 20
     environment:
       EVM: << pipeline.parameters.evm-version >>
       OPTIMIZE: 0
@@ -969,6 +973,7 @@ jobs:
 
   t_ubu_asan_clang_soltest:
     <<: *base_ubuntu2004_clang
+    parallelism: 20
     environment:
       EVM: << pipeline.parameters.evm-version >>
       OPTIMIZE: 0
@@ -978,6 +983,7 @@ jobs:
 
   t_ubu_ubsan_clang_soltest:
     <<: *base_ubuntu2004_clang
+    parallelism: 20
     environment:
       EVM: << pipeline.parameters.evm-version >>
     <<: *steps_soltest

.circleci/soltest.sh

@@ -50,19 +50,48 @@ mkdir -p test_results
 ulimit -s 16384
 
 get_logfile_basename() {
+    local run="$1"
     local filename="${EVM}"
     test "${OPTIMIZE}" = "1" && filename="${filename}_opt"
     test "${ABI_ENCODER_V1}" = "1" && filename="${filename}_abiv1"
+    filename="${filename}_${run}"
 
     echo -ne "${filename}"
 }
 
-BOOST_TEST_ARGS=("--color_output=no" "--show_progress=yes" "--logger=JUNIT,error,test_results/$(get_logfile_basename).xml" "${BOOST_TEST_ARGS[@]}")
+[ -z "$CIRCLE_NODE_TOTAL" ] || [ "$CIRCLE_NODE_TOTAL" = 0 ] && CIRCLE_NODE_TOTAL=1
-SOLTEST_ARGS=("--evm-version=$EVM" "${SOLTEST_FLAGS[@]}")
+[ -z "$CIRCLE_NODE_INDEX" ] && CIRCLE_NODE_INDEX=0
+[ -z "$INDEX_SHIFT" ] && INDEX_SHIFT=0
 
-test "${OPTIMIZE}" = "1" && SOLTEST_ARGS+=(--optimize)
+# Multiply by a prime number to get better spread, just in case
-test "${ABI_ENCODER_V1}" = "1" && SOLTEST_ARGS+=(--abiencoderv1)
+# long-running test cases are next to each other.
+CIRCLE_NODE_INDEX=$(((CIRCLE_NODE_INDEX + 23 * INDEX_SHIFT) % CIRCLE_NODE_TOTAL))
 
-echo "Running ${REPODIR}/build/test/soltest ${BOOST_TEST_ARGS[*]} -- ${SOLTEST_ARGS[*]}"
+CPUs=3
+PIDs=()
+for run in $(seq 0 $((CPUs - 1)))
+do
+    BOOST_TEST_ARGS_RUN=(
+        "--color_output=no"
+        "--show_progress=yes"
+        "--logger=JUNIT,error,test_results/$(get_logfile_basename "$run").xml"
+        "${BOOST_TEST_ARGS[@]}"
+    )
+    SOLTEST_ARGS=("--evm-version=$EVM" "${SOLTEST_FLAGS[@]}")
 
-"${REPODIR}/build/test/soltest" "${BOOST_TEST_ARGS[@]}" -- "${SOLTEST_ARGS[@]}"
+    test "${OPTIMIZE}" = "1" && SOLTEST_ARGS+=(--optimize)
+    test "${ABI_ENCODER_V1}" = "1" && SOLTEST_ARGS+=(--abiencoderv1)
+
+    BATCH_ARGS=("--batches" "$((CPUs * CIRCLE_NODE_TOTAL))" "--selected-batch" "$((CPUs * CIRCLE_NODE_INDEX + run))")
+
+    echo "Running ${REPODIR}/build/test/soltest ${BOOST_TEST_ARGS_RUN[*]} -- ${SOLTEST_ARGS[*]}"
+
+    "${REPODIR}/build/test/soltest" -l test_suite "${BOOST_TEST_ARGS_RUN[@]}" -- "${SOLTEST_ARGS[@]}" "${BATCH_ARGS[@]}" &
+    PIDs+=($!)
+done
+
+# wait for individual processes to get their exit status
+for pid in ${PIDs[*]}
+do
+    wait "$pid"
+done

.circleci/soltest_all.sh

@@ -31,30 +31,21 @@ REPODIR="$(realpath "$(dirname "$0")"/..)"
 # shellcheck source=scripts/common.sh
 source "${REPODIR}/scripts/common.sh"
 
-# NOTE: If you add/remove values, remember to update `parallelism` setting in CircleCI config.
 EVM_VALUES=(homestead byzantium constantinople petersburg istanbul berlin london)
 DEFAULT_EVM=london
 [[ " ${EVM_VALUES[*]} " =~ $DEFAULT_EVM ]]
 OPTIMIZE_VALUES=(0 1)
-STEPS=$(( 1 + ${#EVM_VALUES[@]} * ${#OPTIMIZE_VALUES[@]} ))
-
-RUN_STEPS=$(circleci_select_steps "$(seq "$STEPS")")
-printTask "Running steps $RUN_STEPS..."
-
-STEP=1
-
 
 # Run for ABI encoder v1, without SMTChecker tests.
-if circleci_step_selected "$RUN_STEPS" "$STEP"
+EVM="${DEFAULT_EVM}" \
-then
+OPTIMIZE=1 \
-    EVM="${DEFAULT_EVM}" \
+ABI_ENCODER_V1=1 \
-    OPTIMIZE=1 \
+BOOST_TEST_ARGS="-t !smtCheckerTests" \
-    ABI_ENCODER_V1=1 \
+"${REPODIR}/.circleci/soltest.sh"
-    BOOST_TEST_ARGS="-t !smtCheckerTests" \
-    "${REPODIR}/.circleci/soltest.sh"
-fi
-((++STEP))
 
+# We shift the batch index so that long-running tests
+# do not always run in the same executor for all EVM versions
+INDEX_SHIFT=0
 for OPTIMIZE in "${OPTIMIZE_VALUES[@]}"
 do
     for EVM in "${EVM_VALUES[@]}"
@@ -68,16 +59,13 @@ do
         DISABLE_SMTCHECKER=""
         [ "${OPTIMIZE}" != "0" ] && DISABLE_SMTCHECKER="-t !smtCheckerTests"
 
-        if circleci_step_selected "$RUN_STEPS" "$STEP"
+        EVM="$EVM" \
-        then
+        OPTIMIZE="$OPTIMIZE" \
-            EVM="$EVM" \
+        SOLTEST_FLAGS="$SOLTEST_FLAGS $ENFORCE_GAS_ARGS $EWASM_ARGS" \
-            OPTIMIZE="$OPTIMIZE" \
+        BOOST_TEST_ARGS="-t !@nooptions $DISABLE_SMTCHECKER" \
-            SOLTEST_FLAGS="$SOLTEST_FLAGS $ENFORCE_GAS_ARGS $EWASM_ARGS" \
+        INDEX_SHIFT="$INDEX_SHIFT" \
-            BOOST_TEST_ARGS="-t !@nooptions $DISABLE_SMTCHECKER" \
+        "${REPODIR}/.circleci/soltest.sh"
-            "${REPODIR}/.circleci/soltest.sh"
-        fi
-        ((++STEP))
-    done
-done
 
-((STEP == STEPS + 1)) || assertFail "Step counter not properly adjusted!"
+        INDEX_SHIFT=$((INDEX_SHIFT + 1))
+    done
+done

test/Common.cpp

@@ -102,6 +102,8 @@ void CommonOptions::addOptions()
 		("testpath", po::value<fs::path>(&this->testPath)->default_value(solidity::test::testPath()), "path to test files")
 		("vm", po::value<std::vector<fs::path>>(&vmPaths), "path to evmc library, can be supplied multiple times.")
 		("ewasm", po::bool_switch(&ewasm)->default_value(ewasm), "tries to automatically find an ewasm vm and enable ewasm test-execution.")
+		("batches", po::value<size_t>(&this->batches)->default_value(1), "set number of batches to split the tests into")
+		("selected-batch", po::value<size_t>(&this->selectedBatch)->default_value(0), "zero-based number of batch to execute")
 		("no-semantic-tests", po::bool_switch(&disableSemanticTests)->default_value(disableSemanticTests), "disable semantic tests")
 		("no-smt", po::bool_switch(&disableSMT)->default_value(disableSMT), "disable SMT checker")
 		("optimize", po::bool_switch(&optimize)->default_value(optimize), "enables optimization")
@@ -126,6 +128,17 @@ void CommonOptions::validate() const
 		ConfigException,
 		"Invalid test path specified."
 	);
+	assertThrow(
+		batches > 0,
+		ConfigException,
+		"Batches needs to be at least 1."
+	);
+	assertThrow(
+		selectedBatch < batches,
+		ConfigException,
+		"Selected batch has to be less than number of batches."
+	);
+
 	if (enforceGasTest)
 	{
 		assertThrow(

test/Common.h

@@ -20,6 +20,7 @@
 
 #include <libsolutil/Exceptions.h>
 #include <liblangutil/EVMVersion.h>
+#include <liblangutil/Exceptions.h>
 
 #include <test/evmc/evmc.h>
 
@@ -67,6 +68,8 @@ struct CommonOptions
 	bool useABIEncoderV1 = false;
 	bool showMessages = false;
 	bool showMetadata = false;
+	size_t batches = 1;
+	size_t selectedBatch = 0;
 
 	langutil::EVMVersion evmVersion() const;
 
@@ -96,4 +99,27 @@ bool isValidSemanticTestPath(boost::filesystem::path const& _testPath);
 
 bool loadVMs(CommonOptions const& _options);
 
+/**
+ * Component to help with splitting up all tests into batches.
+ */
+class Batcher
+{
+public:
+	Batcher(size_t _offset, size_t _batches):
+		m_offset(_offset),
+		m_batches(_batches)
+	{
+		solAssert(m_batches > 0 && m_offset < m_batches);
+	}
+	Batcher(Batcher const&) = delete;
+	Batcher& operator=(Batcher const&) = delete;
+
+	bool checkAndAdvance() { return (m_counter++) % m_batches == m_offset; }
+
+private:
+	size_t const m_offset;
+	size_t const m_batches;
+	size_t m_counter = 0;
+};
+
 }

test/boostTest.cpp

@@ -30,6 +30,7 @@
 #pragma warning(disable:4535) // calling _set_se_translator requires /EHa
 #endif
 #include <boost/test/unit_test.hpp>
+#include <boost/test/tree/traverse.hpp>
 #if defined(_MSC_VER)
 #pragma warning(pop)
 #endif
@@ -60,6 +61,41 @@ void removeTestSuite(std::string const& _name)
 	master.remove(id);
 }
 
+/**
+ * Class that traverses the boost test tree and removes unit tests that are
+ * not in the current batch.
+ */
+class BoostBatcher: public test_tree_visitor
+{
+public:
+	BoostBatcher(solidity::test::Batcher& _batcher):
+		m_batcher(_batcher)
+	{}
+
+	void visit(test_case const& _testCase) override
+	{
+		if (!m_batcher.checkAndAdvance())
+			// disabling them would be nicer, but it does not work like this:
+			// const_cast<test_case&>(_testCase).p_run_status.value = test_unit::RS_DISABLED;
+			m_path.back()->remove(_testCase.p_id);
+	}
+	bool test_suite_start(test_suite const& _testSuite) override
+	{
+		m_path.push_back(&const_cast<test_suite&>(_testSuite));
+		return test_tree_visitor::test_suite_start(_testSuite);
+	}
+	void test_suite_finish(test_suite const& _testSuite) override
+	{
+		m_path.pop_back();
+		test_tree_visitor::test_suite_finish(_testSuite);
+	}
+
+private:
+	solidity::test::Batcher& m_batcher;
+	std::vector<test_suite*> m_path;
+};
+
+
 void runTestCase(TestCase::Config const& _config, TestCase::TestCaseCreator const& _testCaseCreator)
 {
 	try
@@ -100,7 +136,8 @@ int registerTests(
 	bool _enforceViaYul,
 	bool _enforceCompileToEwasm,
 	vector<string> const& _labels,
-	TestCase::TestCaseCreator _testCaseCreator
+	TestCase::TestCaseCreator _testCaseCreator,
+	solidity::test::Batcher& _batcher
 )
 {
 	int numTestsAdded = 0;
@@ -131,33 +168,38 @@ int registerTests(
 					_enforceViaYul,
 					_enforceCompileToEwasm,
 					_labels,
-					_testCaseCreator
+					_testCaseCreator,
+					_batcher
 				);
 		_suite.add(sub_suite);
 	}
 	else
 	{
-		// This must be a vector of unique_ptrs because Boost.Test keeps the equivalent of a string_view to the filename
+		// TODO would be better to set the test to disabled.
-		// that is passed in. If the strings were stored directly in the vector, pointers/references to them would be
+		if (_batcher.checkAndAdvance())
-		// invalidated on reallocation.
+		{
-		static vector<unique_ptr<string const>> filenames;
+			// This must be a vector of unique_ptrs because Boost.Test keeps the equivalent of a string_view to the filename
+			// that is passed in. If the strings were stored directly in the vector, pointers/references to them would be
+			// invalidated on reallocation.
+			static vector<unique_ptr<string const>> filenames;
 
-		filenames.emplace_back(make_unique<string>(_path.string()));
+			filenames.emplace_back(make_unique<string>(_path.string()));
-		auto test_case = make_test_case(
+			auto test_case = make_test_case(
-			[config, _testCaseCreator]
+				[config, _testCaseCreator]
-			{
+				{
-				BOOST_REQUIRE_NO_THROW({
+					BOOST_REQUIRE_NO_THROW({
-					runTestCase(config, _testCaseCreator);
+						runTestCase(config, _testCaseCreator);
-				});
+					});
-			},
+				},
-			_path.stem().string(),
+				_path.stem().string(),
-			*filenames.back(),
+				*filenames.back(),
-			0
+				0
-		);
+			);
-		for (auto const& _label: _labels)
+			for (auto const& _label: _labels)
-			test_case->add_label(_label);
+				test_case->add_label(_label);
-		_suite.add(test_case);
+			_suite.add(test_case);
-		numTestsAdded = 1;
+			numTestsAdded = 1;
+		}
 	}
 	return numTestsAdded;
 }
@@ -172,6 +214,7 @@ void initializeOptions()
 
 	solidity::test::CommonOptions::setSingleton(std::move(options));
 }
+
 }
 
 // TODO: Prototype -- why isn't this declared in the boost headers?
@@ -180,6 +223,8 @@ test_suite* init_unit_test_suite( int /*argc*/, char* /*argv*/[] );
 
 test_suite* init_unit_test_suite( int /*argc*/, char* /*argv*/[] )
 {
+	using namespace solidity::test;
+
 	master_test_suite_t& master = framework::master_test_suite();
 	master.p_name.value = "SolidityTests";
 
@@ -194,6 +239,14 @@ test_suite* init_unit_test_suite( int /*argc*/, char* /*argv*/[] )
 	if (!solidity::test::CommonOptions::get().enforceGasTest)
 		cout << endl << "WARNING :: Gas Cost Expectations are not being enforced" << endl << endl;
 
+	Batcher batcher(CommonOptions::get().selectedBatch, CommonOptions::get().batches);
+	if (CommonOptions::get().batches > 1)
+		cout << "Batch " << CommonOptions::get().selectedBatch << " out of " << CommonOptions::get().batches << endl;
+
+	// Batch the boost tests
+	BoostBatcher boostBatcher(batcher);
+	traverse_test_tree(master, boostBatcher, true);
+
 	// Include the interactive tests in the automatic tests as well
 	for (auto const& ts: g_interactiveTestsuites)
 	{
@@ -205,15 +258,19 @@ test_suite* init_unit_test_suite( int /*argc*/, char* /*argv*/[] )
 		if (ts.needsVM && solidity::test::CommonOptions::get().disableSemanticTests)
 			continue;
 
-		solAssert(registerTests(
+		//TODO
+		//solAssert(
+		registerTests(
 			master,
 			options.testPath / ts.path,
 			ts.subpath,
 			options.enforceViaYul,
 			options.enforceCompileToEwasm,
 			ts.labels,
-			ts.testCaseCreator
+			ts.testCaseCreator,
-		) > 0, std::string("no ") + ts.title + " tests found");
+			batcher
+		);
+		// > 0, std::string("no ") + ts.title + " tests found");
 	}
 
 	if (solidity::test::CommonOptions::get().disableSemanticTests)

test/tools/isoltest.cpp

@@ -119,7 +119,8 @@ public:
 		TestCreator _testCaseCreator,
 		TestOptions const& _options,
 		fs::path const& _basepath,
-		fs::path const& _path
+		fs::path const& _path,
+		solidity::test::Batcher& _batcher
 	);
 private:
 	enum class Request
@@ -269,7 +270,8 @@ TestStats TestTool::processPath(
 	TestCreator _testCaseCreator,
 	TestOptions const& _options,
 	fs::path const& _basepath,
-	fs::path const& _path
+	fs::path const& _path,
+	solidity::test::Batcher& _batcher
 )
 {
 	std::queue<fs::path> paths;
@@ -298,6 +300,11 @@ TestStats TestTool::processPath(
 			++testCount;
 			paths.pop();
 		}
+		else if (!_batcher.checkAndAdvance())
+		{
+			paths.pop();
+			++skippedCount;
+		}
 		else
 		{
 			++testCount;
@@ -373,7 +380,8 @@ std::optional<TestStats> runTestSuite(
 	TestOptions const& _options,
 	fs::path const& _basePath,
 	fs::path const& _subdirectory,
-	string const& _name
+	string const& _name,
+	solidity::test::Batcher& _batcher
 )
 {
 	fs::path testPath{_basePath / _subdirectory};
@@ -389,7 +397,8 @@ std::optional<TestStats> runTestSuite(
 		_testCaseCreator,
 		_options,
 		_basePath,
-		_subdirectory
+		_subdirectory,
+		_batcher
 	);
 
 	if (stats.skippedCount != stats.testCount)
@@ -415,21 +424,23 @@ std::optional<TestStats> runTestSuite(
 
 int main(int argc, char const *argv[])
 {
+	using namespace solidity::test;
+
 	try
 	{
 		setupTerminal();
 
 		{
-			auto options = std::make_unique<solidity::test::IsolTestOptions>();
+			auto options = std::make_unique<IsolTestOptions>();
 
 			if (!options->parse(argc, argv))
 				return -1;
 
 			options->validate();
-			solidity::test::CommonOptions::setSingleton(std::move(options));
+			CommonOptions::setSingleton(std::move(options));
 		}
 
-		auto& options = dynamic_cast<solidity::test::IsolTestOptions const&>(solidity::test::CommonOptions::get());
+		auto& options = dynamic_cast<IsolTestOptions const&>(CommonOptions::get());
 
 		if (!solidity::test::loadVMs(options))
 			return 1;
@@ -443,6 +454,10 @@ int main(int argc, char const *argv[])
 		TestStats global_stats{0, 0};
 		cout << "Running tests..." << endl << endl;
 
+		Batcher batcher(CommonOptions::get().selectedBatch, CommonOptions::get().batches);
+		if (CommonOptions::get().batches > 1)
+			cout << "Batch " << CommonOptions::get().selectedBatch << " out of " << CommonOptions::get().batches << endl;
+
 		// Actually run the tests.
 		// Interactive tests are added in InteractiveTests.h
 		for (auto const& ts: g_interactiveTestsuites)
@@ -458,7 +473,8 @@ int main(int argc, char const *argv[])
 				options,
 				options.testPath / ts.path,
 				ts.subpath,
-				ts.title
+				ts.title,
+				batcher
 			);
 			if (stats)
 				global_stats += *stats;