Update all the E2E should_fail tests to verify their output. (#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.

Author: otrho

Cargo.lock

@@ -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"

test/Cargo.toml

@@ -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/{}",

test/src/e2e_vm_tests/harness.rs

@@ -10,7 +10,7 @@ use std::{
     path::{Path, PathBuf},
 };
 
-#[derive(Debug)]
+#[derive(PartialEq)]
 enum TestCategory {
     Compiles,
     FailsToCompile,
@@ -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>) {
@@ -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
@@ -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;
             }
 
@@ -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}"))?;
 
@@ -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)?)
@@ -268,6 +294,7 @@ fn parse_test_toml(path: &Path) -> Result<TestDescription, String> {
         expected_result,
         contract_paths,
         validate_abi,
+        checker,
     })
 }
 

test/src/e2e_vm_tests/mod.rs

@@ -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:
 
@@ -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`.
@@ -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
@@ -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
@@ -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.

test/src/e2e_vm_tests/test_programs/README.md

@@ -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)].

test/src/e2e_vm_tests/test_programs/should_fail/abi_impl_purity_mismatch/test.toml

@@ -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.

test/src/e2e_vm_tests/test_programs/should_fail/abi_method_signature_mismatch/test.toml

@@ -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.

test/src/e2e_vm_tests/test_programs/should_fail/abi_pure_calls_impure/test.toml

@@ -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.

test/src/e2e_vm_tests/test_programs/should_fail/abort_control_flow_bad/test.toml

@@ -1 +1,6 @@
 category = "fail"
+
+# check: ary[false]
+# nextln: $()Mismatched types.
+# nextln: $()expected: u64
+# nextln: $()found:    bool.