Merge pull request #22 from mandreyel/fix/ignore-geth-failing-tests

fix: skip test cases failing in geth

Author: sorpaas

ethjson/src/uint.rs

@@ -82,19 +82,25 @@ impl<'a> Visitor<'a> for UintVisitor {
 	where
 		E: Error,
 	{
+		let parse = |value: &str| {
+			if value.len() > 64 {
+				return Err(Error::custom(
+					format!(
+						"Invalid hex value 0x{value}: value too big (length={})",
+						value.len()
+					)
+					.as_str(),
+				));
+			}
+			U256::from_str(value)
+				.map_err(|e| Error::custom(format!("Invalid hex value 0x{value}: {e}").as_str()))
+		};
+
 		let value = match value.len() {
 			0 => U256::from(0),
 			2 if value.starts_with("0x") => U256::from(0),
-			_ if value.starts_with("0x") => {
-				if value.len() > 66 {
-					return Err(Error::custom(
-						format!("Invalid hex value {}: value too big", value).as_str(),
-					));
-				}
-				U256::from_str(&value[2..]).map_err(|e| {
-					Error::custom(format!("Invalid hex value {}: {}", value, e).as_str())
-				})?
-			}
+			_ if value.starts_with("0x:bigint 0x") => parse(&value[12..])?,
+			_ if value.starts_with("0x") => parse(&value[2..])?,
 			_ => U256::from_dec_str(value).map_err(|e| {
 				Error::custom(format!("Invalid decimal value {}: {:?}", value, e).as_str())
 			})?,
@@ -177,7 +183,7 @@ mod test {
 		assert_eq!(
 			err.to_string(),
 			format!(
-				"Invalid hex value {}: value too big at line 1 column 69",
+				"Invalid hex value {}: value too big (length=65) at line 1 column 69",
 				hex
 			)
 		);

jsontests/src/state.rs

@@ -261,19 +261,18 @@ fn test_run(name: &str, test: Test) {
 			// Test case may be expected to fail with an unsupported tx type if the current fork is
 			// older than Berlin (see EIP-2718). However, this is not implemented in sputnik itself and rather
 			// in the code hosting sputnik. https://github.com/rust-blockchain/evm/pull/40
-			let tx_type = TxType::from_txbytes(&state.txbytes);
-			if matches!(
-				spec,
-				ForkSpec::EIP150
-					| ForkSpec::EIP158 | ForkSpec::Frontier
-					| ForkSpec::Homestead
-					| ForkSpec::Byzantium
-					| ForkSpec::Constantinople
-					| ForkSpec::ConstantinopleFix
-					| ForkSpec::Istanbul
-			) && tx_type != TxType::Legacy
-				&& state.expect_exception == Some("TR_TypeNotSupported".to_string())
-			{
+			let expect_tx_type_not_supported =
+				matches!(
+					spec,
+					ForkSpec::EIP150
+						| ForkSpec::EIP158 | ForkSpec::Frontier
+						| ForkSpec::Homestead | ForkSpec::Byzantium
+						| ForkSpec::Constantinople
+						| ForkSpec::ConstantinopleFix
+						| ForkSpec::Istanbul
+				) && TxType::from_txbytes(&state.txbytes) != TxType::Legacy
+					&& state.expect_exception.as_deref() == Some("TR_TypeNotSupported");
+			if expect_tx_type_not_supported {
 				continue;
 			}
 

jsontests/tests/state.rs

@@ -1,8 +1,8 @@
 use evm_jsontests::state as statetests;
-use std::collections::HashMap;
 use std::fs::{self, File};
 use std::io::BufReader;
 use std::path::PathBuf;
+use std::{collections::HashMap, path::Path};
 
 pub fn run(dir: &str) {
 	let _ = env_logger::try_init();
@@ -20,18 +20,57 @@ pub fn run(dir: &str) {
 
 		let path = entry.path();
 
-		let file = File::open(path).expect("Open file failed");
+		if should_skip(&path) {
+			println!("Skipping test case {path:?}");
+			continue;
+		}
+
+		let file = File::open(&path).expect("Open file failed");
 
 		let reader = BufReader::new(file);
-		let coll: HashMap<String, statetests::Test> =
-			serde_json::from_reader(reader).expect("Parse test cases failed");
+		let coll: HashMap<String, statetests::Test> = serde_json::from_reader(reader)
+			.unwrap_or_else(|e| {
+				panic!("Parsing test case {:?} failed: {:?}", path, e);
+			});
 
 		for (name, test) in coll {
 			statetests::test(&name, test);
 		}
 	}
 }
 
+// NOTE: Add a comment here explaining why you're skipping a test case.
+const SKIPPED_CASES: &[&str] = &[
+	// This is an expected failure case for testing that the VM rejects
+	// transactions with values that are too large, but it's geth
+	// specific because geth parses the hex string later in the test
+	// run, whereas this test runner parses everything up-front before
+	// running the test.
+	"stTransactionTest/ValueOverflow",
+	// The below test cases are failing in geth too and as such are
+	// skipped here until they are fixed there (otherwise we don't know
+	// what the expected value should be for each test output).
+	"stTransactionTest/HighGasPrice",
+	"stCreateTest/CreateTransactionHighNonce",
+];
+
+fn should_skip(path: &Path) -> bool {
+	let matches = |case: &str| {
+		let file_stem = path.file_stem().unwrap();
+		let dir_path = path.parent().unwrap();
+		let dir_name = dir_path.file_name().unwrap();
+		Path::new(dir_name).join(file_stem) == Path::new(case)
+	};
+
+	for case in SKIPPED_CASES {
+		if matches(case) {
+			return true;
+		}
+	}
+
+	false
+}
+
 #[test]
 fn st_args_zero_one_balance() {
 	run("res/ethtests/GeneralStateTests/stArgsZeroOneBalance")