fixup! Migrate ethdebug CLI tests to isolest

Author: nikola-matic

test/CMakeLists.txt

@@ -89,6 +89,8 @@ set(libsolidity_sources
     libsolidity/GasTest.h
     libsolidity/Imports.cpp
     libsolidity/InlineAssembly.cpp
+    libsolidity/JSONExpectationTest.cpp
+    libsolidity/JSONExpectationTest.h
     libsolidity/LibSolc.cpp
     libsolidity/Metadata.cpp
     libsolidity/MemoryGuardTest.cpp
@@ -102,8 +104,6 @@ set(libsolidity_sources
     libsolidity/SemVerMatcher.cpp
     libsolidity/SMTCheckerTest.cpp
     libsolidity/SMTCheckerTest.h
-    libsolidity/StandardJSONTest.cpp
-    libsolidity/StandardJSONTest.h
     libsolidity/SolidityCompiler.cpp
     libsolidity/SolidityEndToEndTest.cpp
     libsolidity/SolidityExecutionFramework.cpp

test/libsolidity/EthdebugTest.cpp

@@ -33,7 +33,7 @@ using namespace solidity::frontend::test;
 using namespace solidity::langutil;
 
 EthdebugTest::EthdebugTest(std::string const& _filename):
-	StandardJSONTest(_filename)
+	JSONExpectationTest(_filename)
 {
 	m_optimise = m_reader.boolSetting("optimize", false);
 	m_optimiseYul = m_reader.boolSetting("optimize-yul", false);

test/libsolidity/EthdebugTest.h

@@ -18,7 +18,7 @@
 
 #pragma once
 
-#include <test/libsolidity/StandardJSONTest.h>
+#include <test/libsolidity/JSONExpectationTest.h>
 
 #include <libsolidity/interface/DebugSettings.h>
 
@@ -42,7 +42,7 @@ namespace solidity::frontend::test
 ///   - Per contract: `Contract.creation`, `Contract.runtime`, `Contract.contract`.
 ///   - Source-qualified per contract (when needed to disambiguate same-named
 ///     contracts in different sources): `source.sol:Contract.creation`, etc.
-class EthdebugTest: public StandardJSONTest
+class EthdebugTest: public JSONExpectationTest
 {
 public:
 	static std::unique_ptr<TestCase> create(Config const& _config)

test/libsolidity/StandardJSONTest.cpp test/libsolidity/JSONExpectationTest.cpptest/libsolidity/StandardJSONTest.cpp renamed to test/libsolidity/JSONExpectationTest.cpp

@@ -16,7 +16,7 @@
 */
 // SPDX-License-Identifier: GPL-3.0
 
-#include <test/libsolidity/StandardJSONTest.h>
+#include <test/libsolidity/JSONExpectationTest.h>
 #include <test/libsolidity/util/Common.h>
 
 #include <libsolutil/CommonIO.h>
@@ -33,14 +33,14 @@ using namespace solidity::frontend::test;
 using namespace solidity::util;
 using namespace std::string_literals;
 
-StandardJSONTest::StandardJSONTest(std::string const& _filename):
+JSONExpectationTest::JSONExpectationTest(std::string const& _filename):
 	EVMVersionRestrictedTestCase(_filename),
 	m_sources(m_reader.sources())
 {
 	parseExpectations(m_reader.stream());
 }
 
-void StandardJSONTest::parseExpectations(std::istream& _stream)
+void JSONExpectationTest::parseExpectations(std::istream& _stream)
 {
 	std::string line;
 	while (getline(_stream, line))
@@ -75,7 +75,10 @@ void StandardJSONTest::parseExpectations(std::istream& _stream)
 	}
 }
 
-std::string StandardJSONTest::extractExpectationJSON(std::istream& _stream)
+// TODO: Current implementation has quadratic complexity due to Json::accept()-ing check after every line is consumed.
+//       It's only an issue for multi line object matching, especially when it comes to very large objects. Should be
+//       swapped to a custom parser if it becomes problematic performance wise at some point.
+std::string JSONExpectationTest::extractExpectationJSON(std::istream& _stream)
 {
 	std::string rawJSON;
 	std::string line;
@@ -100,7 +103,7 @@ std::string StandardJSONTest::extractExpectationJSON(std::istream& _stream)
 	));
 }
 
-StandardJSONTest::Expectation StandardJSONTest::parseExpectationLine(std::string_view _line)
+JSONExpectationTest::Expectation JSONExpectationTest::parseExpectationLine(std::string_view _line)
 {
 	auto const colonPos = _line.find(": ");
 	if (colonPos == std::string_view::npos)
@@ -116,7 +119,7 @@ StandardJSONTest::Expectation StandardJSONTest::parseExpectationLine(std::string
 	return result;
 }
 
-StandardJSONTest::Expectation StandardJSONTest::parsePathPart(std::string_view _pathPart)
+JSONExpectationTest::Expectation JSONExpectationTest::parsePathPart(std::string_view _pathPart)
 {
 	Expectation result;
 
@@ -131,7 +134,7 @@ StandardJSONTest::Expectation StandardJSONTest::parsePathPart(std::string_view _
 	return result;
 }
 
-std::pair<std::string, std::string> StandardJSONTest::resolveOutputKey(std::string_view _path) const
+std::pair<std::string, std::string> JSONExpectationTest::resolveOutputKey(std::string_view _path) const
 {
 	if (_path.empty())
 		BOOST_THROW_EXCEPTION(std::runtime_error("Empty path in expectation"));
@@ -149,7 +152,7 @@ std::pair<std::string, std::string> StandardJSONTest::resolveOutputKey(std::stri
 	return {std::string(scope), std::string(jsonPath)};
 }
 
-std::pair<std::string_view, std::string_view> StandardJSONTest::splitNonGlobalPath(std::string_view _path) const
+std::pair<std::string_view, std::string_view> JSONExpectationTest::splitNonGlobalPath(std::string_view _path) const
 {
 	// Default: first segment is the scope key, the rest is the JSON path.
 	auto const firstDot = _path.find('.');
@@ -180,41 +183,46 @@ nlohmann::json::json_pointer pathToPointer(std::string_view _path)
 }
 }
 
-std::optional<Json> StandardJSONTest::resolvePath(Json const& _json, std::string const& _path) const
+std::optional<Json> JSONExpectationTest::resolvePath(Json const& _json, std::string const& _path) const
 {
 	auto const ptr = pathToPointer(_path);
 	if (!_json.contains(ptr))
 		return std::nullopt;
 	return _json.at(ptr);
 }
 
-Json StandardJSONTest::applyFilter(Json const& _json, std::string const& _filter) const
+Json JSONExpectationTest::applyFilter(Json const& _json, std::string const& _filter) const
 {
 	if (_filter == "length")
 	{
-		if (_json.is_array())
-			return Json(static_cast<uint64_t>(_json.size()));
-		if (_json.is_object())
+		if (_json.is_array() || _json.is_object())
 			return Json(static_cast<uint64_t>(_json.size()));
 		if (_json.is_string())
 			return Json(static_cast<uint64_t>(_json.get<std::string>().size()));
-		solAssert(false, "Cannot apply 'length' filter to this JSON type");
+		BOOST_THROW_EXCEPTION(std::runtime_error(
+			"'length' filter requires an array, object, or string; got "s + _json.type_name()
+		));
 	}
 	if (_filter == "type")
 		return Json(_json.type_name());
 	if (_filter == "keys")
 	{
-		solAssert(_json.is_object(), "Cannot apply 'keys' filter to non-object");
+		if (!_json.is_object())
+			BOOST_THROW_EXCEPTION(std::runtime_error(
+				"'keys' filter requires an object; got "s + _json.type_name()
+			));
 		Json result = Json::array();
+		// Json uses std::map for object storage, so items() iterates keys in
+		// lexicographic order. Tests rely on this for stable output.
 		for (auto const& [key, _]: _json.items())
 			result.push_back(key);
 		return result;
 	}
 
-	solAssert(false, "Unknown filter: " + _filter);
+	BOOST_THROW_EXCEPTION(std::runtime_error("Unknown filter: " + _filter));
 }
 
-std::string StandardJSONTest::formatValue(Json const& _json) const
+std::string JSONExpectationTest::formatValue(Json const& _json) const
 {
 	if (_json.is_string())
 		return _json.get<std::string>();
@@ -223,12 +231,12 @@ std::string StandardJSONTest::formatValue(Json const& _json) const
 	return _json.dump();
 }
 
-bool StandardJSONTest::isPlaceholder(std::string const& _value) const
+bool JSONExpectationTest::isPlaceholder(std::string const& _value) const
 {
 	return placeholders().count(_value) > 0;
 }
 
-void StandardJSONTest::applyPlaceholders(Json const& _expected, Json& _obtained) const
+void JSONExpectationTest::applyPlaceholders(Json const& _expected, Json& _obtained) const
 {
 	if (_expected.type() != _obtained.type())
 		return;
@@ -251,39 +259,39 @@ void StandardJSONTest::applyPlaceholders(Json const& _expected, Json& _obtained)
 	}
 }
 
-TestCase::TestResult StandardJSONTest::run(std::ostream& _stream, std::string const& _linePrefix, bool _formatted)
+TestCase::TestResult JSONExpectationTest::run(std::ostream& _stream, std::string const& _linePrefix, bool _formatted)
 {
 	if (!runFramework(withPreamble(m_sources.sources), PipelineStage::Compilation))
 	{
 		printPrefixed(_stream, formatErrors(filteredErrors(false /* _includeWarningsAndInfos */), _formatted), _linePrefix);
 		return TestResult::FatalError;
 	}
 
-	auto const outputs = collectScopes();
+	auto const scopeOutputs = collectScopes();
 	bool allMatch = true;
 
 	for (auto const& expectation: m_expectations)
 	{
-		auto const [outputKey, jsonPath] = resolveOutputKey(expectation.fullPath);
-		auto const it = outputs.find(outputKey);
-		if (it == outputs.end())
+		auto const [scopeKey, jsonPath] = resolveOutputKey(expectation.fullPath);
+		auto const scopeIt = scopeOutputs.find(scopeKey);
+		if (scopeIt == scopeOutputs.end())
 		{
 			allMatch = false;
 			continue;
 		}
 
-		auto const resolved = resolvePath(it->second, jsonPath);
-		if (!resolved)
+		auto const actualLookup = resolvePath(scopeIt->second, jsonPath);
+		if (!actualLookup)
 		{
 			allMatch = false;
 			continue;
 		}
 
-		Json value = *resolved;
+		Json actualJson = *actualLookup;
 		if (!expectation.filter.empty())
-			value = applyFilter(value, expectation.filter);
+			actualJson = applyFilter(actualJson, expectation.filter);
 
-		std::string const obtained = formatValue(value);
+		std::string const obtained = formatValue(actualJson);
 
 		if (expectation.value.find('\n') != std::string::npos)
 		{
@@ -294,7 +302,7 @@ TestCase::TestResult StandardJSONTest::run(std::ostream& _stream, std::string co
 				allMatch = false;
 				continue;
 			}
-			Json patchedObtained = value;
+			Json patchedObtained = actualJson;
 			applyPlaceholders(expectedJson, patchedObtained);
 
 			if (jsonPrint(patchedObtained, {JsonFormat::Pretty, 4}) !=
@@ -303,16 +311,16 @@ TestCase::TestResult StandardJSONTest::run(std::ostream& _stream, std::string co
 		}
 		else if (isPlaceholder(expectation.value))
 		{
-			if (!value.is_string())
+			if (!actualJson.is_string())
 				allMatch = false;
 		}
-		else if (value.is_object() || value.is_array())
+		else if (actualJson.is_object() || actualJson.is_array())
 		{
 			// Compound single-line value: parse expected as JSON and compare structurally
 			Json expectedJson;
 			std::string errors;
 			if (!jsonParseStrict(expectation.value, expectedJson, &errors) ||
-				value != expectedJson)
+				actualJson != expectedJson)
 				allMatch = false;
 		}
 		else if (obtained != expectation.value)
@@ -329,7 +337,7 @@ TestCase::TestResult StandardJSONTest::run(std::ostream& _stream, std::string co
 	return TestResult::Failure;
 }
 
-void StandardJSONTest::printSource(std::ostream& _stream, std::string const& _linePrefix, bool /*_formatted*/) const
+void JSONExpectationTest::printSource(std::ostream& _stream, std::string const& _linePrefix, bool /*_formatted*/) const
 {
 	if (m_sources.sources.empty())
 		return;
@@ -343,7 +351,7 @@ void StandardJSONTest::printSource(std::ostream& _stream, std::string const& _li
 	}
 }
 
-std::string StandardJSONTest::formatExpectations(std::vector<Expectation> const& _expectations) const
+std::string JSONExpectationTest::formatExpectations(std::vector<Expectation> const& _expectations) const
 {
 	std::string output;
 	for (auto const& exp: _expectations)
@@ -363,7 +371,7 @@ std::string StandardJSONTest::formatExpectations(std::vector<Expectation> const&
 	return output;
 }
 
-void StandardJSONTest::printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const
+void JSONExpectationTest::printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const
 {
 	auto const outputs = collectScopes();
 

test/libsolidity/StandardJSONTest.h test/libsolidity/JSONExpectationTest.htest/libsolidity/StandardJSONTest.h renamed to test/libsolidity/JSONExpectationTest.h

@@ -36,30 +36,34 @@ namespace solidity::frontend::test
 /// Base class for isoltest test cases that drive a Solidity compilation and assert
 /// against a JSON output.
 ///
-/// Expectations
-/// ---------------
-/// Single-line:
-///     // path: expected
-///     // path | filter: expected
+/// Each expectation has the form:
+///     // path [| filter]: expected
 ///
-/// Multi-line (for whole objects/arrays):
-///     // path:
+/// `expected` is a literal value or a JSON literal. When the JSON spans multiple
+/// lines, the path ends with `:` and the JSON continues on subsequent `// ` lines.
+///
+/// Example:
+///     // C.instructions[0].offset: 0
+///     // C.instructions | length: 3
+///     // C.instructions | keys: ["context", "mnemonic", "offset"]
+///     // C.compilation:
 ///     // {
-///     //     "expected": "json spread across multiple lines"
+///     //     "compiler": {"name": "solc", "version": "<VERSION>"},
+///     //     "settings": {"optimizer": {"enabled": false}}
 ///     // }
 ///
 /// Filters (applied to the resolved value before comparison):
-///   - `| length`  — array/object/string size as a number
-///   - `| type`    — JSON type name ("object", "array", "number", …)
-///   - `| keys`    — list of keys (object only)
+///   - `| length` — array/object/string size as a number
+///   - `| type`   — JSON type name ("object", "array", "number", …)
+///   - `| keys`   — list of keys (object only); always lexicographically sorted
 ///
-/// Placeholders:
-///   - `<VERSION>` (default) at a leaf matches any string. Subclasses may extend
-///     the recognized set via `placeholders()`.
-class StandardJSONTest: public AnalysisFramework, public EVMVersionRestrictedTestCase
+/// Placeholders at string leaves match any string. Subclasses extend the
+/// recognized set via `placeholders()`. The placeholder name is descriptive
+/// only — content is not validated beyond JSON type.
+class JSONExpectationTest: public AnalysisFramework, public EVMVersionRestrictedTestCase
 {
 public:
-	StandardJSONTest(std::string const& _filename);
+	JSONExpectationTest(std::string const& _filename);
 
 	TestResult run(std::ostream& _stream, std::string const& _linePrefix = "", bool _formatted = false) override;
 	void printSource(std::ostream& _stream, std::string const& _linePrefix = "", bool _formatted = false) const override;

test/tools/CMakeLists.txt

@@ -40,8 +40,8 @@ add_executable(isoltest
 	../libsolidity/ASTJSONTest.cpp
 	../libsolidity/ASTPropertyTest.cpp
 	../libsolidity/FunctionDependencyGraphTest.cpp
+	../libsolidity/JSONExpectationTest.cpp
 	../libsolidity/SMTCheckerTest.cpp
-	../libsolidity/StandardJSONTest.cpp
 	../libyul/Common.cpp
 	../libyul/ControlFlowGraphTest.cpp
 	../libyul/ControlFlowSideEffectsTest.cpp