Skip to content

Commit

Permalink
test: use Result instead of Option in should_skip (#204)
Browse files Browse the repository at this point in the history
  • Loading branch information
DaniPopes authored Feb 7, 2025
1 parent e558f2b commit 98ca2f1
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 24 deletions.
4 changes: 2 additions & 2 deletions tools/tester/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ fn file_filter(path: &Path, config: &ui_test::Config, cfg: MyConfig<'_>) -> Opti
}
let skip = match cfg.mode {
Mode::Ui => false,
Mode::SolcSolidity => solc::solidity::should_skip(path).is_some(),
Mode::SolcYul => solc::yul::should_skip(path).is_some(),
Mode::SolcSolidity => solc::solidity::should_skip(path).is_err(),
Mode::SolcYul => solc::yul::should_skip(path).is_err(),
};
Some(!skip)
}
Expand Down
28 changes: 14 additions & 14 deletions tools/tester/src/solc/solidity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,55 +6,55 @@ use std::{
sync::atomic::{AtomicUsize, Ordering},
};

pub(crate) fn should_skip(path: &Path) -> Option<&'static str> {
pub(crate) fn should_skip(path: &Path) -> Result<(), &'static str> {
let path_contains = path_contains_curry(path);

if path_contains("/libyul/") {
return Some("actually a Yul test");
return Err("actually a Yul test");
}

if path_contains("/cmdlineTests/") {
return Some("CLI tests do not have the same format as everything else");
return Err("CLI tests do not have the same format as everything else");
}

if path_contains("/lsp/") {
return Some("LSP tests do not have the same format as everything else");
return Err("LSP tests do not have the same format as everything else");
}

if path_contains("/ASTJSON/") {
return Some("no JSON AST");
return Err("no JSON AST");
}

if path_contains("/functionDependencyGraphTests/") || path_contains("/experimental") {
return Some("solidity experimental is not implemented");
return Err("solidity experimental is not implemented");
}

// We don't parse licenses.
if path_contains("/license/") {
return Some("licenses are not checked");
return Err("licenses are not checked");
}

if path_contains("natspec") {
return Some("natspec is not checked");
return Err("natspec is not checked");
}

if path_contains("_direction_override") {
return Some("Unicode direction override checks not implemented");
return Err("Unicode direction override checks not implemented");
}

if path_contains("max_depth_reached_") {
return Some("recursion guard will not be implemented");
return Err("recursion guard will not be implemented");
}

if path_contains("wrong_compiler_") {
return Some("Solidity pragma version is not checked");
return Err("Solidity pragma version is not checked");
}

// Directories starting with `_` are not tests.
if path_contains("/_")
&& !path.components().next_back().unwrap().as_os_str().to_str().unwrap().starts_with('_')
{
return Some("supporting file");
return Err("supporting file");
}

let stem = path.file_stem().unwrap().to_str().unwrap();
Expand Down Expand Up @@ -102,10 +102,10 @@ pub(crate) fn should_skip(path: &Path) -> Option<&'static str> {
| "invalid_state_variable_location"
| "location_specifiers_for_state_variables"
) {
return Some("manually skipped");
return Err("manually skipped");
};

None
Ok(())
}

/// Handles `====` and `==== ExternalSource: ... ====` delimiters in a solc test file.
Expand Down
16 changes: 8 additions & 8 deletions tools/tester/src/solc/yul.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use crate::utils::path_contains_curry;
use std::path::Path;

pub(crate) fn should_skip(path: &Path) -> Option<&'static str> {
pub(crate) fn should_skip(path: &Path) -> Result<(), &'static str> {
let path_contains = path_contains_curry(path);

if path_contains("/recursion_depth.yul") {
return Some("recursion stack overflow");
return Err("recursion stack overflow");
}

if path_contains("/verbatim") {
return Some("verbatim Yul builtin is not implemented");
return Err("verbatim Yul builtin is not implemented");
}

if path_contains("/period_in_identifier")
Expand All @@ -19,16 +19,16 @@ pub(crate) fn should_skip(path: &Path) -> Option<&'static str> {
// Why does Solc parse periods as part of Yul identifiers?
// `yul-identifier` is the same as `solidity-identifier`, which disallows periods:
// https://docs.soliditylang.org/en/latest/grammar.html#a4.SolidityLexer.YulIdentifier
return Some("not actually valid identifiers");
return Err("not actually valid identifiers");
}

if path_contains("objects/conflict_") || path_contains("objects/code.yul") {
// Not the parser's job to check conflicting names.
return Some("not implemented in the parser");
return Err("not implemented in the parser");
}

if path_contains(".sol") {
return Some("not a Yul file");
return Err("not a Yul file");
}

let stem = path.file_stem().unwrap().to_str().unwrap();
Expand Down Expand Up @@ -63,8 +63,8 @@ pub(crate) fn should_skip(path: &Path) -> Option<&'static str> {
| "mcopy_pre_cancun"
| "tstore_tload_as_identifiers_pre_cancun"
) {
return Some("manually skipped");
return Err("manually skipped");
};

None
Ok(())
}

0 comments on commit 98ca2f1

Please sign in to comment.