From 98dc5919f4dbfb5a24c0b6be38d04d3a71d2daa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Matos?= Date: Mon, 29 Jul 2024 22:33:03 -0300 Subject: [PATCH] Add `core` and `std` dependency test filtering to the test runner (#6299) ## Description This adds a couple new flags to the test runner which allow filtering the tests to exclude them if they contain a dependency on either `core` or `std`. This is useful to debug issues, since it allows to run tests without any dependencies, making debugging a bit less difficult when dealing with complicated issues. There is also a minor refactoring to `BuildPlan::from_build_opts` which I thought was necessary, but ended up not being used, but I think doesnt hurt to keep. ## Checklist - [ ] I have linked to any relevant issues. - [ ] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [ ] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers. --- forc-pkg/src/pkg.rs | 12 ++++++------ forc-test/src/lib.rs | 6 +++--- forc/src/ops/forc_contract_id.rs | 2 +- test/src/e2e_vm_tests/mod.rs | 27 +++++++++++++++++++++++++++ test/src/main.rs | 12 ++++++++++++ 5 files changed, 49 insertions(+), 10 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 99da8d36402..22b4dc6ed5c 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -582,8 +582,8 @@ impl BuildPlan { /// /// To do so, it tries to read the manifet file at the target path and creates the plan with /// `BuildPlan::from_lock_and_manifest`. - pub fn from_build_opts(build_options: &BuildOpts) -> Result { - let path = &build_options.pkg.path; + pub fn from_pkg_opts(pkg_options: &PkgOpts) -> Result { + let path = &pkg_options.path; let manifest_dir = if let Some(ref path) = path { PathBuf::from(path) @@ -601,9 +601,9 @@ impl BuildPlan { Self::from_lock_and_manifests( &lock_path, &member_manifests, - build_options.pkg.locked, - build_options.pkg.offline, - &build_options.pkg.ipfs_node, + pkg_options.locked, + pkg_options.offline, + &pkg_options.ipfs_node, ) } @@ -2153,7 +2153,7 @@ pub fn build_with_options(build_options: &BuildOpts) -> Result { .as_ref() .map_or_else(|| current_dir, PathBuf::from); - let build_plan = BuildPlan::from_build_opts(build_options)?; + let build_plan = BuildPlan::from_pkg_opts(&build_options.pkg)?; let graph = build_plan.graph(); let manifest_map = build_plan.manifest_map(); diff --git a/forc-test/src/lib.rs b/forc-test/src/lib.rs index 7a2ca0a57bc..e1fca4ac561 100644 --- a/forc-test/src/lib.rs +++ b/forc-test/src/lib.rs @@ -5,7 +5,7 @@ use crate::execute::TestExecutor; use crate::setup::{ ContractDeploymentSetup, ContractTestSetup, DeploymentSetup, ScriptTestSetup, TestSetup, }; -use forc_pkg as pkg; +use forc_pkg::{self as pkg, BuildOpts}; use fuel_abi_types::error_codes::ErrorSignal; use fuel_tx as tx; use fuel_vm::checked_transaction::builder::TransactionBuilderExt; @@ -601,8 +601,8 @@ impl BuiltTests { /// First builds the package or workspace, ready for execution. pub fn build(opts: TestOpts) -> anyhow::Result { - let build_opts = opts.into(); - let build_plan = pkg::BuildPlan::from_build_opts(&build_opts)?; + let build_opts: BuildOpts = opts.into(); + let build_plan = pkg::BuildPlan::from_pkg_opts(&build_opts.pkg)?; let built = pkg::build_with_options(&build_opts)?; BuiltTests::from_built(built, &build_plan) } diff --git a/forc/src/ops/forc_contract_id.rs b/forc/src/ops/forc_contract_id.rs index 51e9a7cd708..f64a73386ee 100644 --- a/forc/src/ops/forc_contract_id.rs +++ b/forc/src/ops/forc_contract_id.rs @@ -8,7 +8,7 @@ use tracing::info; pub fn contract_id(command: ContractIdCommand) -> Result<()> { let build_options = build_opts_from_cmd(&command); - let build_plan = pkg::BuildPlan::from_build_opts(&build_options)?; + let build_plan = pkg::BuildPlan::from_pkg_opts(&build_options.pkg)?; // If a salt was specified but we have more than one member to build, there // may be ambiguity in how the salt should be applied, especially if the // workspace contains multiple contracts, and especially if one contract diff --git a/test/src/e2e_vm_tests/mod.rs b/test/src/e2e_vm_tests/mod.rs index b5ed9a43040..62993a04149 100644 --- a/test/src/e2e_vm_tests/mod.rs +++ b/test/src/e2e_vm_tests/mod.rs @@ -9,6 +9,7 @@ use crate::{FilterConfig, RunConfig}; use anyhow::{anyhow, bail, Result}; use colored::*; use core::fmt; +use forc_pkg::manifest::{GenericManifestFile, ManifestFile}; use forc_pkg::BuildProfile; use forc_test::decode_log_data; use fuel_vm::fuel_tx; @@ -653,6 +654,13 @@ pub async fn run(filter_config: &FilterConfig, run_config: &RunConfig) -> Result .as_ref() .map(|exclude| tests.retained(|t| !exclude.is_match(&t.name))) .unwrap_or_default(); + + if filter_config.exclude_core { + tests.retain(|t| exclude_tests_dependency(t, "core")); + } + if filter_config.exclude_std { + tests.retain(|t| exclude_tests_dependency(t, "std")); + } if filter_config.abi_only { tests.retain(|t| t.validate_abi); } @@ -792,6 +800,25 @@ pub async fn run(filter_config: &FilterConfig, run_config: &RunConfig) -> Result } } +fn exclude_tests_dependency(t: &TestDescription, dep: &str) -> bool { + let manifest_dir = env!("CARGO_MANIFEST_DIR"); + let tests_root_dir = format!("{manifest_dir}/src/e2e_vm_tests/test_programs"); + let file_name = &t.name; + let manifest_path = format!("{tests_root_dir}/{file_name}"); + match ManifestFile::from_dir(manifest_path) { + Ok(manifest_file) => { + let member_manifests = manifest_file.member_manifests().unwrap(); + !member_manifests.iter().any(|(_name, manifest)| { + manifest + .dependencies + .as_ref() + .is_some_and(|map| map.contains_key(dep)) + }) + } + Err(_) => true, + } +} + fn discover_test_configs(run_config: &RunConfig) -> Result> { fn recursive_search( path: &Path, diff --git a/test/src/main.rs b/test/src/main.rs index 856b8e4728c..8dff1419ee7 100644 --- a/test/src/main.rs +++ b/test/src/main.rs @@ -29,6 +29,14 @@ struct Cli { #[arg(long, visible_alias = "abi")] abi_only: bool, + /// Only run tests with no core dependencies + #[arg(long, visible_alias = "exclude_core")] + exclude_core: bool, + + /// Only run tests with no std dependencies + #[arg(long, visible_alias = "exclude_std")] + exclude_std: bool, + /// Only run tests that deploy contracts #[arg(long, visible_alias = "contract")] contract_only: bool, @@ -80,6 +88,8 @@ pub struct FilterConfig { pub exclude: Option, pub skip_until: Option, pub abi_only: bool, + pub exclude_core: bool, + pub exclude_std: bool, pub contract_only: bool, pub first_only: bool, } @@ -108,6 +118,8 @@ async fn main() -> Result<()> { exclude: cli.exclude, skip_until: cli.skip_until, abi_only: cli.abi_only, + exclude_core: cli.exclude_core, + exclude_std: cli.exclude_std, contract_only: cli.contract_only, first_only: cli.first_only, };