Skip to content

Commit

Permalink
Remove Rust integration testing behaviour from forc test command
Browse files Browse the repository at this point in the history
As proposed in #1833, this
removes the integration testing implementation under `forc test` in
anticipation of using the command for unit testing.
  • Loading branch information
mitchmindtree committed Oct 9, 2022
1 parent 61d6d65 commit 859ef1b
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 234 deletions.
4 changes: 0 additions & 4 deletions forc-util/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,6 @@ pub fn find_parent_dir_with_file(starter_path: &Path, file_name: &str) -> Option
pub fn find_manifest_dir(starter_path: &Path) -> Option<PathBuf> {
find_parent_dir_with_file(starter_path, constants::MANIFEST_FILE_NAME)
}
/// Continually go up in the file tree until a Cargo manifest file is found.
pub fn find_cargo_manifest_dir(starter_path: &Path) -> Option<PathBuf> {
find_parent_dir_with_file(starter_path, constants::TEST_MANIFEST_FILE_NAME)
}

pub fn is_sway_file(file: &Path) -> bool {
let res = file.extension();
Expand Down
4 changes: 1 addition & 3 deletions forc/src/cli/commands/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ use crate::ops::forc_clean;
use anyhow::Result;
use clap::Parser;

/// Removes the default forc compiler output artifact directory, i.e. `<project-name>/out`. Also
/// calls `cargo clean` which removes the `target` directory generated by `cargo` when running
/// tests.
/// Removes the default forc compiler output artifact directory, i.e. `<project-name>/out`.
#[derive(Debug, Parser)]
pub struct Command {
/// Path to the project, if not specified, current working directory will be used.
Expand Down
123 changes: 30 additions & 93 deletions forc/src/cli/commands/test.rs
Original file line number Diff line number Diff line change
@@ -1,104 +1,41 @@
use crate::ops::forc_build;
use anyhow::{bail, Result};
use clap::Parser;
use std::io::{BufRead, BufReader};
use std::process;
use std::thread;
use tracing::{error, info};

/// Run Rust-based tests on current project.
/// As of now, `forc test` is a simple wrapper on
/// `cargo test`; `forc new` also creates a rust
/// package under your project, named `tests`.
/// You can opt to either run these Rust tests by
/// using `forc test` or going inside the package
/// and using `cargo test`.
/// Run the Sway unit tests for the current project.
///
/// NOTE: This feature is not yet implemented. Track progress at the following link:
/// https://github.com/FuelLabs/sway/issues/1832
///
/// NOTE: Previously this command was used to support Rust integration testing, however the
/// provided behaviour served no benefit over running `cargo test` directly. The proposal to change
/// the behaviour to support unit testing can be found at the following link:
/// https://github.com/FuelLabs/sway/issues/1833
///
/// Sway unit tests are functions decorated with the `#[test_script]` attribute. Each test is
/// compiled as an independent `script` program and has access to the namespace of the module in
/// which it is declared. Unit tests declared within `contract` projects may also call directly
/// into their associated contract's ABI.
///
/// Upon successful compilation, test scripts are executed to their completion. A test is
/// considered a failure in the case that a revert (`rvrt`) instruction is encountered during
/// execution. Otherwise, it is considered a success.
#[derive(Debug, Parser)]
pub(crate) struct Command {
/// If specified, only run tests containing this string in their names
pub test_name: Option<String>,
/// Options passed through to the `cargo test` invocation.
///
/// E.g. Given the following:
///
/// `forc test --cargo-test-opts="--color always"`
///
/// The `--color always` option is forwarded to `cargo test` like so:
///
/// `cargo test --color always`
#[clap(long)]
pub cargo_test_opts: Option<String>,
/// All trailing arguments following `--` are collected within this argument.
///
/// E.g. Given the following:
///
/// `forc test -- foo bar baz`
///
/// The arguments `foo`, `bar` and `baz` are forwarded on to `cargo test` like so:
///
/// `cargo test -- foo bar baz`
#[clap(raw = true)]
pub cargo_test_args: Vec<String>,
/// When specified, only tests containing the given string will be executed.
pub filter: Option<String>,
}

pub(crate) fn exec(command: Command) -> Result<()> {
// Ensure the project builds before running tests.
forc_build::build(Default::default())?;
pub(crate) fn exec(_command: Command) -> Result<()> {
bail!(r#"
Sway unit testing is not yet implemented. Track progress at the following link:
let mut cmd = process::Command::new("cargo");
cmd.arg("test");
https://github.com/FuelLabs/sway/issues/1832
// Pass through cargo test options.
let mut user_specified_color_opt = false;
if let Some(opts) = command.cargo_test_opts {
user_specified_color_opt = opts.contains("--color");
for opt in opts.split_whitespace() {
cmd.arg(&opt);
}
}
NOTE: Previously this command was used to support Rust integration testing,
however the provided behaviour served no benefit over runnin `cargo test`
directly. The proposal to change the behaviour to support unit testing can be
found at the following link:
// If the coloring option wasn't specified by the user, enable it ourselves. This is useful as
// `cargo test`'s coloring is disabled by default when run as a child process.
if !user_specified_color_opt {
cmd.args(&["--color", "always"]);
}

// Pass through test name.
if let Some(ref name) = command.test_name {
cmd.arg(name);
}

// Pass through cargo test args.
if !command.cargo_test_args.is_empty() {
cmd.arg("--");
cmd.args(&command.cargo_test_args);
}

let mut child = cmd
.stdout(process::Stdio::piped())
.stderr(process::Stdio::piped())
.spawn()
.unwrap();

let out = BufReader::new(child.stdout.take().unwrap());
let err = BufReader::new(child.stderr.take().unwrap());

// Reading stderr on a separate thread so we keep things non-blocking
let thread = thread::spawn(move || {
err.lines().for_each(|line| error!("{}", line.unwrap()));
});

out.lines().for_each(|line| info!("{}", line.unwrap()));
thread.join().unwrap();

let child_success = match child.try_wait() {
Ok(Some(returned_status)) => returned_status.success(),
Ok(None) => child.wait().unwrap().success(),
Err(_) => false,
};

match child_success {
true => Ok(()),
false => bail!("child test process failed"),
}
https://github.com/FuelLabs/sway/issues/1833
"#);
}
14 changes: 2 additions & 12 deletions forc/src/ops/forc_clean.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::cli::CleanCommand;
use anyhow::{anyhow, bail, Result};
use forc_util::{default_output_directory, find_cargo_manifest_dir, find_manifest_dir};
use std::{path::PathBuf, process};
use forc_util::{default_output_directory, find_manifest_dir};
use std::path::PathBuf;
use sway_utils::MANIFEST_FILE_NAME;

pub fn clean(command: CleanCommand) -> Result<()> {
Expand Down Expand Up @@ -29,15 +29,5 @@ pub fn clean(command: CleanCommand) -> Result<()> {
let out_dir = default_output_directory(&manifest_dir);
let _ = std::fs::remove_dir_all(out_dir);

// Run `cargo clean`, forwarding stdout and stderr (`cargo clean` doesn't appear to output
// anything as of writing this).
if find_cargo_manifest_dir(&this_dir).is_some() {
process::Command::new("cargo")
.arg("clean")
.stderr(process::Stdio::inherit())
.stdout(process::Stdio::inherit())
.output()?;
}

Ok(())
}
18 changes: 0 additions & 18 deletions forc/src/ops/forc_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@ pub fn init(command: InitCommand) -> Result<()> {
// Make a new directory for the project
fs::create_dir_all(Path::new(&project_dir).join("src"))?;

// Make directory for tests
fs::create_dir_all(Path::new(&project_dir).join("tests"))?;

// Insert default manifest file
match program_type {
Library => fs::write(
Expand All @@ -104,12 +101,6 @@ pub fn init(command: InitCommand) -> Result<()> {
)?,
}

// Insert default test manifest file
fs::write(
Path::new(&project_dir).join(constants::TEST_MANIFEST_FILE_NAME),
defaults::default_tests_manifest(&project_name),
)?;

match program_type {
Contract => fs::write(
Path::new(&project_dir)
Expand Down Expand Up @@ -137,15 +128,6 @@ pub fn init(command: InitCommand) -> Result<()> {
)?,
}

// Insert default test function
let harness_path = Path::new(&project_dir).join("tests").join("harness.rs");
fs::write(&harness_path, defaults::default_test_program(&project_name))?;

debug!(
"\nCreated test harness at {}",
harness_path.canonicalize()?.display()
);

// Ignore default `out` and `target` directories created by forc and cargo.
let gitignore_path = Path::new(&project_dir).join(".gitignore");
// Append to existing gitignore if it exists otherwise create a new one.
Expand Down
16 changes: 1 addition & 15 deletions forc/src/ops/forc_template.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::cli::TemplateCommand;
use crate::utils::defaults;
use anyhow::{anyhow, Context, Result};
use forc_pkg::{
fetch_git, fetch_id, find_dir_within, git_commit_path, pin_git, PackageManifest, SourceGit,
Expand All @@ -9,7 +8,7 @@ use fs_extra::dir::{copy, CopyOptions};
use std::fs::File;
use std::io::{Read, Write};
use std::path::{Path, PathBuf};
use std::{env, fs};
use std::env;
use sway_utils::constants;
use tracing::info;
use url::Url;
Expand Down Expand Up @@ -73,19 +72,6 @@ pub fn init(command: TemplateCommand) -> Result<()> {
edit_forc_toml(&target_dir, &command.project_name, &whoami::realname())?;
if target_dir.join("test").exists() {
edit_cargo_toml(&target_dir, &command.project_name, &whoami::realname())?;
} else {
// Create the tests directory, harness.rs and Cargo.toml file
fs::create_dir_all(target_dir.join("tests"))?;

fs::write(
target_dir.join("tests").join("harness.rs"),
defaults::default_test_program(&command.project_name),
)?;

fs::write(
target_dir.join("Cargo.toml"),
defaults::default_tests_manifest(&command.project_name),
)?;
}
Ok(())
}
Expand Down
89 changes: 0 additions & 89 deletions forc/src/utils/defaults.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,6 @@ name = "{project_name}"
)
}

/// Creates a default Cargo manifest for the Rust-based tests.
/// It includes necessary packages to make the Rust-based
/// tests work.
pub(crate) fn default_tests_manifest(project_name: &str) -> String {
let author = get_author();

format!(
r#"[project]
name = "{project_name}"
version = "0.1.0"
authors = ["{author}"]
edition = "2021"
license = "Apache-2.0"
[dependencies]
fuels = {{ version = "0.25", features = ["fuel-core-lib"] }}
tokio = {{ version = "1.12", features = ["rt", "macros"] }}
[[test]]
harness = true
name = "integration_tests"
path = "tests/harness.rs"
"#
)
}

pub(crate) fn default_contract() -> String {
r#"contract;
Expand Down Expand Up @@ -86,61 +60,6 @@ fn main() -> bool {
.into()
}

// TODO Ideally after (instance, id) it should link to the The Fuels-rs Book
// to provide further information for writing tests/working with sway
pub(crate) fn default_test_program(project_name: &str) -> String {
format!(
"{}{}{}{}{}{}{}",
r#"use fuels::{prelude::*, tx::ContractId};
// Load abi from json
abigen!(MyContract, "out/debug/"#,
project_name,
r#"-abi.json");
async fn get_contract_instance() -> (MyContract, ContractId) {
// Launch a local network and deploy the contract
let mut wallets = launch_custom_provider_and_get_wallets(
WalletsConfig::new(
Some(1), /* Single wallet */
Some(1), /* Single coin (UTXO) */
Some(1_000_000_000), /* Amount per coin */
),
None,
)
.await;
let wallet = wallets.pop().unwrap();
let id = Contract::deploy(
"./out/debug/"#,
project_name,
r#".bin",
&wallet,
TxParameters::default(),
StorageConfiguration::with_storage_path(Some(
"./out/debug/"#,
project_name,
r#"-storage_slots.json".to_string(),
)),
)
.await
.unwrap();
let instance = MyContract::new(id.to_string(), wallet);
(instance, id.into())
}
#[tokio::test]
async fn can_get_contract_id() {
let (_instance, _id) = get_contract_instance().await;
// Now you have an instance of your contract you can use to test each function
}
"#
)
}

pub(crate) fn default_gitignore() -> String {
r#"out
target
Expand All @@ -161,11 +80,3 @@ fn parse_default_manifest() {
.unwrap()
)
}

#[test]
fn parse_default_tests_manifest() {
tracing::info!(
"{:#?}",
toml::from_str::<forc_pkg::PackageManifest>(&default_tests_manifest("test_proj")).unwrap()
)
}

0 comments on commit 859ef1b

Please sign in to comment.