Skip to content

Commit

Permalink
Add core and std dependency test filtering to the test runner (#6299
Browse files Browse the repository at this point in the history
)

## 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.
  • Loading branch information
tritao authored Jul 30, 2024
1 parent ec01af4 commit 98dc591
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 10 deletions.
12 changes: 6 additions & 6 deletions forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self> {
let path = &build_options.pkg.path;
pub fn from_pkg_opts(pkg_options: &PkgOpts) -> Result<Self> {
let path = &pkg_options.path;

let manifest_dir = if let Some(ref path) = path {
PathBuf::from(path)
Expand All @@ -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,
)
}

Expand Down Expand Up @@ -2153,7 +2153,7 @@ pub fn build_with_options(build_options: &BuildOpts) -> Result<Built> {
.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();

Expand Down
6 changes: 3 additions & 3 deletions forc-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -601,8 +601,8 @@ impl BuiltTests {

/// First builds the package or workspace, ready for execution.
pub fn build(opts: TestOpts) -> anyhow::Result<BuiltTests> {
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)
}
Expand Down
2 changes: 1 addition & 1 deletion forc/src/ops/forc_contract_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions test/src/e2e_vm_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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<Vec<TestDescription>> {
fn recursive_search(
path: &Path,
Expand Down
12 changes: 12 additions & 0 deletions test/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -80,6 +88,8 @@ pub struct FilterConfig {
pub exclude: Option<regex::Regex>,
pub skip_until: Option<regex::Regex>,
pub abi_only: bool,
pub exclude_core: bool,
pub exclude_std: bool,
pub contract_only: bool,
pub first_only: bool,
}
Expand Down Expand Up @@ -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,
};
Expand Down

0 comments on commit 98dc591

Please sign in to comment.