ci: improve test resilience and run style checks within `cargo nextes…

…t` (#9608)

I got absolutely fed up with waiting for prerequisite infrastructure
work for #9290 (why does it have to be _that_ hard?)

However the PR in question had some other important improvements that do
not necessarily *rely* on changes to said infrastructure to work. In
particular:

* `nextest` now retries failing tests a few times before giving up, to
make sure they aren't spuriously failing;
* This should help some timing out integration tests in particular, as
those often fail because of some deadlockish situation in my experience.
* style checks still run with `cargo nextest` – unfortunately that means
that in CI this will run these checks multiple times, but that doesn’t
sound particularly terrible of a tradeoff (especially if the other
changes mean we won't be retrying entire test suites as often anymore;)
* This should allow for a greater portion of the test suite to run on
Macs – unfortunately not verified by the CI, but people do complain and
this should make the situation better.

cc #9367

.config/nextest.toml

@@ -1,6 +1,35 @@
 [profile.default]
 slow-timeout = { period = "60s", terminate-after = 2, grace-period = "0s" }
+# FIXME(nagisa): use --profile ci in CI instead when we manage to modify CI scripts...
+retries = { backoff = "fixed", count = 3, delay = "1s" }
+failure-output = "final"
 
 [[profile.default.overrides]]
 filter = 'test(test_full_estimator)'
 slow-timeout = { period = "10m", terminate-after = 3 }
+threads-required = 4
+
+[[profile.default.overrides]]
+filter = 'package(style-tests)'
+slow-timeout = { period = "120s", terminate-after = 5 }
+threads-required = 4
+
+# Unfortunately no support for inheriting profiles yet:
+# https://github.com/nextest-rs/nextest/issues/387
+[profile.ci]
+slow-timeout = { period = "120s", terminate-after = 5 }
+# Try a few times before failing the whole test suite on a potentially spurious tests.
+# The hope is that people will fix the spurious tests as they encounter them locally...
+retries = { backoff = "fixed", count = 3, delay = "1s" }
+failure-output = "final"
+fail-fast = false
+
+[[profile.ci.overrides]]
+filter = 'test(test_full_estimator)'
+slow-timeout = { period = "10m", terminate-after = 3 }
+threads-required = 4
+
+[[profile.ci.overrides]]
+filter = 'package(style-tests)'
+slow-timeout = { period = "120s", terminate-after = 5 }
+threads-required = 4

Cargo.toml

@@ -65,6 +65,7 @@ members = [
     "test-utils/runtime-tester/fuzz",
     "test-utils/store-validator",
     "test-utils/testlib",
+    "test-utils/style",
     "tools/database",
     "tools/chainsync-loadtest",
     "tools/fork-network",

core/o11y/src/io_tracer.rs

@@ -117,7 +117,7 @@ impl<S: Subscriber + for<'span> LookupSpan<'span>> Layer<S> for IoTraceLayer {
         }
     }
 
-    #[allow(clippy::integer_arithmetic)]
+    #[allow(clippy::arithmetic_side_effects)]
     fn on_exit(&self, id: &span::Id, ctx: tracing_subscriber::layer::Context<'_, S>) {
         // When the span exits, produce one line for the span itself that
         // includes key=value pairs from `SpanInfo`. Then also add indentation
@@ -311,7 +311,7 @@ impl tracing::field::Visit for IoEventVisitor {
 }
 
 impl tracing::field::Visit for SpanInfo {
-    #[allow(clippy::integer_arithmetic)]
+    #[allow(clippy::arithmetic_side_effects)]
     fn record_str(&mut self, field: &tracing::field::Field, value: &str) {
         // "count" is a special field, everything else are key values pairs.
         if field.name() == "counter" {

core/o11y/src/testonly.rs

@@ -31,24 +31,24 @@ fn setup_subscriber_from_filter(mut env_filter: EnvFilter) {
 }
 
 pub fn init_test_logger() {
-    let env_filter = EnvFilter::new("tokio_reactor=info,tokio_core=info,hyper=info,debug");
+    let env_filter = EnvFilter::new("cranelift=warn,wasmtime=warn,h2=warn,tower=warn,trust_dns=warn,tokio_reactor=info,tokio_core=info,hyper=info,debug");
     setup_subscriber_from_filter(env_filter);
 }
 
 pub fn init_test_logger_allow_panic() {
-    let env_filter = EnvFilter::new("tokio_reactor=info,tokio_core=info,hyper=info,debug");
+    let env_filter = EnvFilter::new("cranelift=warn,wasmtime=warn,h2=warn,tower=warn,trust_dns=warn,tokio_reactor=info,tokio_core=info,hyper=info,debug");
     setup_subscriber_from_filter(env_filter);
 }
 
 pub fn init_test_module_logger(module: &str) {
     let env_filter =
-        EnvFilter::new("tokio_reactor=info,tokio_core=info,hyper=info,cranelift_wasm=warn,info")
+        EnvFilter::new("cranelift=warn,wasmtime=warn,h2=warn,tower=warn,trust_dns=warn,tokio_reactor=info,tokio_core=info,hyper=info,cranelift_wasm=warn,info")
             .add_directive(format!("{}=info", module).parse().unwrap());
     setup_subscriber_from_filter(env_filter);
 }
 
 pub fn init_integration_logger() {
-    let env_filter = EnvFilter::new("actix_web=warn,info");
+    let env_filter = EnvFilter::new("cranelift=warn,wasmtime=warn,actix_web=warn,info");
     setup_subscriber_from_filter(env_filter);
 }
 

integration-tests/src/tests/client/epoch_sync.rs

@@ -24,7 +24,7 @@ fn generate_transactions(last_hash: &CryptoHash, h: BlockHeight) -> Vec<SignedTr
             vec![Action::DeployContract(DeployContractAction {
                 code: near_test_contracts::rs_contract().to_vec(),
             })],
-            last_hash.clone(),
+            *last_hash,
         ));
     }
 
@@ -40,7 +40,7 @@ fn generate_transactions(last_hash: &CryptoHash, h: BlockHeight) -> Vec<SignedTr
                 gas: 100_000_000_000_000,
                 deposit: 0,
             }))],
-            last_hash.clone(),
+            *last_hash,
         ));
     }
 
@@ -51,7 +51,7 @@ fn generate_transactions(last_hash: &CryptoHash, h: BlockHeight) -> Vec<SignedTr
             "test1".parse().unwrap(),
             &signer,
             1,
-            last_hash.clone(),
+            *last_hash,
         ));
     }
     txs

runtime/near-vm-runner/src/tests.rs

@@ -8,7 +8,6 @@ pub(crate) mod test_builder;
 mod ts_contract;
 mod wasm_validation;
 
-#[cfg(all(feature = "near_vm", target_arch = "x86_64"))]
 use crate::config::ContractPrepareVersion;
 use crate::logic::{Config, VMContext};
 use crate::vm_kind::VMKind;

runtime/near-vm-runner/src/tests/fuzzers.rs

@@ -158,6 +158,7 @@ fn current_vm_does_not_crash() {
 }
 
 #[test]
+#[cfg_attr(not(target_arch = "x86_64"), ignore)]
 fn wasmer2_and_wasmtime_agree() {
     check!().for_each(|data: &[u8]| {
         let module = ArbitraryModule::arbitrary(&mut arbitrary::Unstructured::new(data));
@@ -174,6 +175,7 @@ fn wasmer2_and_wasmtime_agree() {
 }
 
 #[test]
+#[cfg_attr(not(all(feature = "near_vm", target_arch = "x86_64")), ignore)]
 fn near_vm_and_wasmtime_agree() {
     check!().for_each(|data: &[u8]| {
         let module = ArbitraryModule::arbitrary(&mut arbitrary::Unstructured::new(data));
@@ -188,8 +190,8 @@ fn near_vm_and_wasmtime_agree() {
     });
 }
 
-#[cfg(all(feature = "near_vm", target_arch = "x86_64"))]
 #[test]
+#[cfg(all(feature = "near_vm", target_arch = "x86_64"))]
 fn near_vm_is_reproducible() {
     use crate::near_vm_runner::NearVM;
     use near_primitives::hash::CryptoHash;

runtime/near-vm-runner/src/tests/test_builder.rs

@@ -29,12 +29,18 @@ pub(crate) fn test_builder() -> TestBuilder {
         view_config: None,
         output_data_receivers: vec![],
     };
+    let mut skip = HashSet::new();
+    if cfg!(not(target_arch = "x86_64")) {
+        skip.insert(VMKind::Wasmer0);
+        skip.insert(VMKind::Wasmer2);
+        skip.insert(VMKind::NearVm);
+    }
     TestBuilder {
         code: ContractCode::new(Vec::new(), None),
         context,
         method: "main".to_string(),
         protocol_versions: vec![u32::MAX],
-        skip: HashSet::new(),
+        skip,
         opaque_error: false,
         opaque_outcome: false,
     }

runtime/near-vm/test-api/src/lib.rs

@@ -1,2 +1,4 @@
+#![cfg(target_arch = "x86_64")]
+
 mod sys;
 pub use sys::*;

runtime/near-vm/tests/compilers/main.rs

@@ -1,6 +1,6 @@
+#![cfg(target_arch = "x86_64")]
 //! This test suite does all the tests that involve any compiler
 //! implementation, such as: singlepass.
-
 #[macro_use]
 extern crate near_vm_compiler_test_derive;
 

runtime/near-vm/wast/src/lib.rs

@@ -1,5 +1,5 @@
 //! Implementation of the WAST text format for wasmer.
-
+#![cfg(target_arch = "x86_64")]
 #![deny(missing_docs, trivial_numeric_casts, unused_extern_crates)]
 #![warn(unused_import_braces)]
 #![deny(unstable_features)]

rust-toolchain.toml

@@ -3,5 +3,5 @@
 # Individual crates in the workspace may support a lower version, as indicated by `rust-version` field in each crate's `Cargo.toml`.
 # The version specified below, should be at least as high as the maximum `rust-version` within the workspace.
 channel = "1.72.0"
-components = [ "rustfmt" ]
+components = [ "rustfmt", "clippy" ]
 targets = [ "wasm32-unknown-unknown" ]

test-utils/style/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "style-tests"
+version = "0.0.0"
+edition = "2021"
+publish = false
+description = "An entry point for miscelaneous (stylistic) workspace-wide tests"
+authors = ["Near Inc <hello@nearprotocol.com>"]
+
+[dependencies]

test-utils/style/src/lib.rs

@@ -0,0 +1,82 @@
+#![cfg(test)]
+use std::{
+    ffi::{OsStr, OsString},
+    path::PathBuf,
+    process::Command,
+};
+
+/// Add common cargo arguments for tests run by this code.
+fn cargo_env(cmd: &mut Command) {
+    // Set the working directory to the project root, rather than using whatever default nextest
+    // gives us.
+    let style_root = std::env::var_os("CARGO_MANIFEST_DIR").unwrap_or(OsString::from("./"));
+    let wp_root: PathBuf = [&style_root, OsStr::new(".."), OsStr::new("..")].into_iter().collect();
+    cmd.current_dir(&wp_root);
+
+    // Use a different target directory to avoid invalidating any cache after tests are run (so
+    // that running `cargo nextest` twice does not rebuild half of the workspace on the 2nd
+    // rebuild. Unfortunately cargo itself does not readily expose this information to us, so we
+    // have to guess a little as to where this directory might end up.
+    //
+    // NB: We aren't using a temporary directory proper here in order to *allow* keeping cache
+    // between individual `clippy` runs and such.
+    let target_dir: PathBuf =
+        [wp_root.as_os_str(), OsStr::new("target"), OsStr::new("style")].into_iter().collect();
+    cmd.env("CARGO_TARGET_DIR", target_dir.as_path());
+}
+
+fn ensure_success(mut cmd: std::process::Command) {
+    println!("Running {:?}", cmd);
+    match cmd.status() {
+        Err(e) => {
+            panic!("Could not spawn the command: {e}")
+        }
+        Ok(out) if !out.success() => panic!("exit code {:?}", out),
+        Ok(_) => {}
+    }
+}
+
+#[test]
+fn rustfmt() {
+    let cargo = std::env::var_os("CARGO").unwrap_or(OsString::from("cargo"));
+    let mut cmd = Command::new(cargo);
+    cargo_env(&mut cmd);
+    cmd.args(&["fmt", "--", "--check"]);
+    ensure_success(cmd);
+}
+
+#[test]
+fn clippy() {
+    let cargo = std::env::var_os("CARGO").unwrap_or(OsString::from("cargo"));
+    let mut cmd = Command::new(cargo);
+    cargo_env(&mut cmd);
+    cmd.args(&["clippy", "--all-targets", "--all-features", "--"]);
+    cmd.args(&[
+        "-Aclippy::all",
+        "-Dclippy::clone_on_copy",
+        "-Dclippy::correctness",
+        "-Dclippy::derivable_impls",
+        "-Dclippy::redundant_clone",
+        "-Dclippy::suspicious",
+        "-Dclippy::len_zero",
+    ]);
+    ensure_success(cmd);
+}
+
+#[test]
+fn deny() {
+    let cargo = std::env::var_os("CARGO").unwrap_or(OsString::from("cargo"));
+    let mut cmd = Command::new(cargo);
+    cargo_env(&mut cmd);
+    cmd.args(&["deny", "--all-features", "check", "bans"]);
+    ensure_success(cmd);
+}
+
+#[test]
+fn themis() {
+    let cargo = std::env::var_os("CARGO").unwrap_or(OsString::from("cargo"));
+    let mut cmd = Command::new(cargo);
+    cargo_env(&mut cmd);
+    cmd.args(&["run", "-p", "themis"]);
+    ensure_success(cmd);
+}