Skip to content

Commit

Permalink
Update all the E2E should_fail tests to verify their output. (FuelLab…
Browse files Browse the repository at this point in the history
…s#2082)

Using the `FileCheck` crate it can now pattern match against the
compiler output to be sure the errors and/or warnings are exactly what
we expect.
  • Loading branch information
otrho authored Jun 23, 2022
1 parent 289f301 commit 2c5564b
Show file tree
Hide file tree
Showing 95 changed files with 627 additions and 33 deletions.
23 changes: 23 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ publish = false

[dependencies]
anyhow = "1.0.41"
filecheck = "0.5"
forc = { path = "../forc", features = ["test"], default-features = false }
forc-util = { path = "../forc-util" }
fuel-asm = "0.5"
fuel-tx = "0.12"
fuel-vm = { version = "0.11", features = ["random"] }
gag = "1.0"
rand = "0.8"
regex = "1"
serde_json = "1.0.73"
Expand Down
47 changes: 38 additions & 9 deletions test/src/e2e_vm_tests/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,22 +98,51 @@ pub(crate) fn runs_in_vm(file_name: &str, locked: bool) -> ProgramState {
*i.transact(tx_to_test).unwrap().state()
}

/// Panics if code _does_ compile, used for test cases where the source
/// code should have been rejected by the compiler.
pub(crate) fn does_not_compile(file_name: &str, locked: bool) {
assert!(
compile_to_bytes(file_name, locked).is_err(),
"{} should not have compiled.",
file_name,
)
/// Returns Err(()) if code _does_ compile, used for test cases where the source
/// code should have been rejected by the compiler. When it fails to compile the
/// captured stdout is returned.
pub(crate) fn does_not_compile(file_name: &str, locked: bool) -> Result<String, ()> {
use std::io::Read;

tracing::info!(" Compiling {}", file_name);

// Capture stdout to a buffer, compile the test and save stdout to a string.
let mut buf = gag::BufferRedirect::stdout().unwrap();
let result = compile_to_bytes_verbose(file_name, locked, true);
let mut output = String::new();
buf.read_to_string(&mut output).unwrap();
drop(buf);

// If verbosity is requested then print it out.
if get_test_config_from_env() {
tracing::info!("{output}");
}

// Invert the result; if it succeeds then return an Err.
match result {
Ok(_) => Err(()),
Err(e) => {
// Capture the result of the compilation (i.e., any errors Forc produces) and append to
// the stdout from the compiler.
output.push_str(&format!("\n{e}"));
Ok(output)
}
}
}

/// Returns `true` if a file compiled without any errors or warnings,
/// and `false` if it did not.
pub(crate) fn compile_to_bytes(file_name: &str, locked: bool) -> Result<Vec<u8>> {
compile_to_bytes_verbose(file_name, locked, get_test_config_from_env())
}

pub(crate) fn compile_to_bytes_verbose(
file_name: &str,
locked: bool,
verbose: bool,
) -> Result<Vec<u8>> {
tracing::info!(" Compiling {}", file_name);
let manifest_dir = env!("CARGO_MANIFEST_DIR");
let verbose = get_test_config_from_env();
forc_build::build(BuildCommand {
path: Some(format!(
"{}/src/e2e_vm_tests/test_programs/{}",
Expand Down
39 changes: 33 additions & 6 deletions test/src/e2e_vm_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::{
path::{Path, PathBuf},
};

#[derive(Debug)]
#[derive(PartialEq)]
enum TestCategory {
Compiles,
FailsToCompile,
Expand All @@ -27,13 +27,13 @@ enum TestResult {
Revert(u64),
}

#[derive(Debug)]
struct TestDescription {
name: String,
category: TestCategory,
expected_result: Option<TestResult>,
contract_paths: Vec<String>,
validate_abi: bool,
checker: filecheck::Checker,
}

pub fn run(locked: bool, filter_regex: Option<regex::Regex>) {
Expand All @@ -55,6 +55,7 @@ pub fn run(locked: bool, filter_regex: Option<regex::Regex>) {
expected_result,
contract_paths,
validate_abi,
checker,
} in configured_tests
{
if !filter_regex
Expand Down Expand Up @@ -91,7 +92,20 @@ pub fn run(locked: bool, filter_regex: Option<regex::Regex>) {
}

TestCategory::FailsToCompile => {
crate::e2e_vm_tests::harness::does_not_compile(&name, locked);
match crate::e2e_vm_tests::harness::does_not_compile(&name, locked) {
Ok(output) => match checker.explain(&output, filecheck::NO_VARIABLES) {
Ok((success, report)) if !success => {
panic!("For {name}:\nFilecheck failed:\n{report}");
}
Err(e) => {
panic!("For {name}:\nFilecheck directive error: {e}");
}
_ => (),
},
Err(_) => {
panic!("For {name}:\nFailing test did not fail.");
}
}
number_of_tests_executed += 1;
}

Expand Down Expand Up @@ -191,12 +205,19 @@ fn discover_test_configs() -> Result<Vec<TestDescription>, String> {
}

fn parse_test_toml(path: &Path) -> Result<TestDescription, String> {
let toml_content = std::fs::read_to_string(path)
let (toml_content, checker) = std::fs::read_to_string(path)
.map_err(|e| e.to_string())
.and_then(|toml_content_str| {
toml_content_str
.parse::<toml::Value>()
// Parse the file for FileCheck directives and_then parse the file into TOML.
filecheck::CheckerBuilder::new()
.text(&toml_content_str)
.map_err(|e| e.to_string())
.and_then(|checker| {
toml_content_str
.parse::<toml::Value>()
.map_err(|e| e.to_string())
.map(|toml_content| (toml_content, checker.finish()))
})
})
.map_err(|e| format!("Failed to parse: {e}"))?;

Expand All @@ -219,6 +240,11 @@ fn parse_test_toml(path: &Path) -> Result<TestDescription, String> {
Some(other) => Err(format!("Unknown category '{}'.", other,)),
})?;

// Abort early if we find a FailsToCompile test without any Checker directives.
if category == TestCategory::FailsToCompile && checker.is_empty() {
return Err("'fail' tests must contain some FileCheck verification directives.".to_owned());
}

let expected_result = match &category {
TestCategory::Runs | TestCategory::RunsWithContract => {
Some(get_expected_result(&toml_content)?)
Expand Down Expand Up @@ -268,6 +294,7 @@ fn parse_test_toml(path: &Path) -> Result<TestDescription, String> {
expected_result,
contract_paths,
validate_abi,
checker,
})
}

Expand Down
43 changes: 38 additions & 5 deletions test/src/e2e_vm_tests/test_programs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
Each of the tests in this suite are controlled by a TOML descriptor file which describes how the
test should be run and what result to expect if any.

## `test.toml`
## test.toml

To add a new test to the E2E suite place a `test.toml` file at the root of the test Forc package,
i.e., next to the `Forc.toml` file. This file may contain a few basic fields.

## `category`
## category

The `category` field is mandatory and must be one of the following strings:

Expand All @@ -18,7 +18,7 @@ The `category` field is mandatory and must be one of the following strings:
* `"fail"` - The test is expected to fail to compile.
* `"disabled"` - The test is disabled.

## `expected_result`
## expected_result

The `expected_result` field is mandatory for `"run"` and `"run_on_node"` tests. It is a table with
two fields, `action` and `value`.
Expand All @@ -35,7 +35,7 @@ it must be an integer.

For `"return_data"` actions it must be an array of byte values, each an integer between 0 and 255.

## `contracts`
## contracts

Tests in the `"run_on_node"` category will usually specify one or more contracts which must be
deployed to the node prior to deploying and running the test code. These are specified with the
Expand All @@ -45,11 +45,31 @@ It must be an array of strings each containing only the path to the Forc project
be compiled and deployed. It is important that these paths remain relative to the
`test/src/e2e_vm_tests/test_programs` directory.

## `validate_abi`
## validate_abi

Some tests also require their ABI is verified. To indicate this the `validate_abi` field may be
specified, as a boolean value.

# FileCheck for 'fail' tests

The tests in the `fail` category _must_ employ verification using pattern matching via the [FileCheck](https://docs.rs/filecheck/latest/filecheck/)
crate. The checker directives are specified in comments (lines beginning with `#`) in the `test.toml`
file.

Typically this is as simple as just adding a `# check: ...` line to the line specifying the full
error or warning message expected from compiling the test. `FileCheck` also has other directives for
fancier pattern matching, as specified in the [FileCheck docs](https://docs.rs/filecheck/latest/filecheck/).

**Note:** The output from the compiler is colorized, usually to red or yellow, and this involves
printing ANSI escape sequences to the terminal. These sequences can confuse `FileCheck` as it tries
to match patterns on 'word' boundaries. The first word in an error message is most likely prefixed
with an escape sequence and can cause the check to fail.

To avoid this problem one may either not use the first word in the error message, or use the 'empty
string' pattern `$()` to direct the matcher as to where the pattern starts.

E.g, `# check: $()The name "S" shadows another symbol with the same name.`

# Examples

The following is a common example for tests in the `should_pass/language` directory. The test
Expand Down Expand Up @@ -78,5 +98,18 @@ expected_result = { action = "result", value = 11 }
contracts = ["should_pass/test_contracts/test_contract_a", "should_pass/test_contracts/test_contract_b"]
```

Tests which fail can have fairly elaborate checks.

```toml
category = "fail"

# check: // this asm block should return unit, i.e. nothing
# nextln: asm(r1: 5) {
# check: $()Mismatched types.
# nextln: $()expected: ()
# nextln: $()found: u64.
# nextln: $()help: Implicit return must match up with block's type.
```

And there are already hundreds of existing tests with `test.toml` descriptors which may be consulted
when adding a new test.
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
category = "fail"

# check: $()Storage attribute access mismatch. The trait function "test_function" in trait "MyContract" requires the storage attribute(s) #[storage(read)].
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
category = "fail"

# check: fn foo(s: str[7]) -> str[7] {
# nextln: $()Expected: u64
# nextln: $()found: str[7]. The definition of this function must match the one in the trait declaration.
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
category = "fail"

# check: f()
# nextln: $()Storage attribute access mismatch. Try giving the surrounding function more access by adding "#[storage(read)]" to the function declaration.
Original file line number Diff line number Diff line change
@@ -1 +1,16 @@
category = "fail"

# check: return 42;
# nextln: $()Mismatched types.
# nextln: $()expected: ()
# nextln: $()found: u64.
# nextln: $()help: Return statement must return the declared function return type.

# This 'return true' line appears in both error messages..
# check: return true;

# check: return true;
# nextln: $()Mismatched types.
# nextln: $()expected: ()
# nextln: $()found: bool.
# nextln: $()help: Return statement must return the declared function return type.
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
category = "fail"

# check: ary[false]
# nextln: $()Mismatched types.
# nextln: $()expected: u64
# nextln: $()found: bool.
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
category = "fail"

# check: $()Array index out of bounds; the length is 3 but the index is 4.
Original file line number Diff line number Diff line change
@@ -1 +1,13 @@
category = "fail"

# check: asm(r1: 5) {
# check: $()Mismatched types.
# nextln: $()expected: u64
# nextln: $()found: ().
# nextln: $()help: Implicit return must match up with block's type.

# check: asm(r1: 5) {
# check: $()Mismatched types.
# nextln: $()expected: u64
# nextln: $()found: ().
# nextln: $()help: Function body's return type does not match up with its return type annotation.
Original file line number Diff line number Diff line change
@@ -1 +1,15 @@
category = "fail"

# check: // this asm block should return unit, i.e. nothing
# nextln: asm(r1: 5) {
# check: $()Mismatched types.
# nextln: $()expected: ()
# nextln: $()found: u64.
# nextln: $()help: Implicit return must match up with block's type.

# check: // this asm block should return unit, i.e. nothing
# nextln: asm(r1: 5) {
# check: $()Mismatched types.
# nextln: $()expected: ()
# nextln: $()found: u64.
# nextln: $()help: Function body's return type does not match up with its return type annotation.
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
category = "fail"

# check: $()Assignment to immutable variable. Variable thing is not declared as mutable.
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
category = "fail"

# check: let g: u32 = three_generics(true, "foo", 10);
# nextln: $()Mismatched types.
# nextln: $()expected: u32
# nextln: $()found: str[3].
# nextln: $()help: Variable declaration's type annotation does not match up with the assigned expression's type.
Loading

0 comments on commit 2c5564b

Please sign in to comment.