Skip to content

Commit

Permalink
revert #1086
Browse files Browse the repository at this point in the history
  • Loading branch information
@transmissions11
transmissions11 committed Mar 28, 2022
1 parent 867e2d8 commit 3835e52
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 58 deletions.
32 changes: 10 additions & 22 deletions cli/src/cmd/forge/snapshot.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,25 +88,24 @@ impl Cmd for SnapshotArgs {
type Output = ();

fn run(self) -> eyre::Result<()> {
let include_fuzz_test_gas = self.test.include_fuzz_test_gas;
let outcome = self.test.run()?;
outcome.ensure_ok(include_fuzz_test_gas)?;
outcome.ensure_ok()?;
let tests = self.config.apply(outcome);

if let Some(path) = self.diff {
let snap = path.as_ref().unwrap_or(&self.snap);
let snaps = read_snapshot(snap)?;
diff(tests, include_fuzz_test_gas, snaps)?;
diff(tests, snaps)?;
} else if let Some(path) = self.check {
let snap = path.as_ref().unwrap_or(&self.snap);
let snaps = read_snapshot(snap)?;
if check(tests, include_fuzz_test_gas, snaps) {
if check(tests, snaps) {
std::process::exit(0)
} else {
std::process::exit(1)
}
} else {
write_to_snapshot_file(&tests, self.snap, include_fuzz_test_gas, self.format)?;
write_to_snapshot_file(&tests, self.snap, self.format)?;
}
Ok(())
}
Expand Down Expand Up @@ -208,7 +207,6 @@ impl FromStr for SnapshotEntry {
contract_name: file.as_str().to_string(),
signature: sig.as_str().to_string(),
gas_used: TestKindGas::Fuzz {
include_fuzz_test_gas: true,
runs: runs.as_str().parse().unwrap(),
median: med.as_str().parse().unwrap(),
mean: avg.as_str().parse().unwrap(),
Expand Down Expand Up @@ -242,7 +240,6 @@ fn read_snapshot(path: impl AsRef<Path>) -> eyre::Result<Vec<SnapshotEntry>> {
fn write_to_snapshot_file(
tests: &[Test],
path: impl AsRef<Path>,
include_fuzz_test_gas: bool,
_format: Option<Format>,
) -> eyre::Result<()> {
let mut out = String::new();
Expand All @@ -252,7 +249,7 @@ fn write_to_snapshot_file(
"{}:{} {}",
test.contract_name(),
test.signature,
test.result.kind.gas_used(include_fuzz_test_gas)
test.result.kind.gas_used()
)?;
}
Ok(fs::write(path, out)?)
Expand Down Expand Up @@ -284,7 +281,7 @@ impl SnapshotDiff {
/// Compares the set of tests with an existing snapshot
///
/// Returns true all tests match
fn check(tests: Vec<Test>, include_fuzz_test_gas: bool, snaps: Vec<SnapshotEntry>) -> bool {
fn check(tests: Vec<Test>, snaps: Vec<SnapshotEntry>) -> bool {
let snaps = snaps
.into_iter()
.map(|s| ((s.contract_name, s.signature), s.gas_used))
Expand All @@ -294,7 +291,7 @@ fn check(tests: Vec<Test>, include_fuzz_test_gas: bool, snaps: Vec<SnapshotEntry
if let Some(target_gas) =
snaps.get(&(test.contract_name().to_string(), test.signature.clone())).cloned()
{
let source_gas = test.result.kind.gas_used(include_fuzz_test_gas);
let source_gas = test.result.kind.gas_used();
if source_gas.gas() != target_gas.gas() {
eprintln!(
"Diff in \"{}::{}\": consumed \"{}\" gas, expected \"{}\" gas ",
Expand All @@ -318,11 +315,7 @@ fn check(tests: Vec<Test>, include_fuzz_test_gas: bool, snaps: Vec<SnapshotEntry
}

/// Compare the set of tests with an existing snapshot
fn diff(
tests: Vec<Test>,
include_fuzz_test_gas: bool,
snaps: Vec<SnapshotEntry>,
) -> eyre::Result<()> {
fn diff(tests: Vec<Test>, snaps: Vec<SnapshotEntry>) -> eyre::Result<()> {
let snaps = snaps
.into_iter()
.map(|s| ((s.contract_name, s.signature), s.gas_used))
Expand All @@ -340,7 +333,7 @@ fn diff(
})?;

diffs.push(SnapshotDiff {
source_gas_used: test.result.kind.gas_used(include_fuzz_test_gas),
source_gas_used: test.result.kind.gas_used(),
signature: test.signature,
target_gas_used,
});
Expand Down Expand Up @@ -420,12 +413,7 @@ mod tests {
SnapshotEntry {
contract_name: "Test".to_string(),
signature: "deposit()".to_string(),
gas_used: TestKindGas::Fuzz {
runs: 256,
median: 200,
mean: 100,
include_fuzz_test_gas: true
}
gas_used: TestKindGas::Fuzz { runs: 256, median: 200, mean: 100 }
}
);
}
Expand Down
39 changes: 14 additions & 25 deletions cli/src/cmd/forge/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,9 @@ pub struct TestArgs {
debug: Option<Regex>,

/// Print a gas report.
#[clap(long, env = "FORGE_GAS_REPORT")]
#[clap(long = "gas-report")]
gas_report: bool,

/// Include the mean and median gas use of fuzz tests in the output.
#[clap(long, env = "FORGE_INCLUDE_FUZZ_TEST_GAS")]
pub include_fuzz_test_gas: bool,

/// Force the process to exit with code 0, even if the tests fail.
#[clap(long, env = "FORGE_ALLOW_FAILURE")]
allow_failure: bool,
Expand Down Expand Up @@ -239,7 +235,7 @@ impl Cmd for TestArgs {
};
debugger.run()?;

Ok(TestOutcome::new(results, self.allow_failure, self.include_fuzz_test_gas))
Ok(TestOutcome::new(results, self.allow_failure))
}
n =>
Err(
Expand All @@ -255,7 +251,7 @@ impl Cmd for TestArgs {
filter,
self.json,
self.allow_failure,
(self.include_fuzz_test_gas, self.gas_report, config.gas_reports),
(self.gas_report, config.gas_reports),
)
}
}
Expand All @@ -275,7 +271,7 @@ pub struct Test {

impl Test {
pub fn gas_used(&self) -> u64 {
self.result.kind.gas_used(true).gas()
self.result.kind.gas_used().gas()
}

/// Returns the contract name of the artifact id
Expand All @@ -293,19 +289,13 @@ impl Test {
pub struct TestOutcome {
/// Whether failures are allowed
pub allow_failure: bool,
/// Whether to include fuzz test gas usage in the output
pub include_fuzz_test_gas: bool,
/// Results for each suite of tests `contract -> SuiteResult`
pub results: BTreeMap<String, SuiteResult>,
}

impl TestOutcome {
fn new(
results: BTreeMap<String, SuiteResult>,
allow_failure: bool,
include_fuzz_test_gas: bool,
) -> Self {
Self { results, include_fuzz_test_gas, allow_failure }
fn new(results: BTreeMap<String, SuiteResult>, allow_failure: bool) -> Self {
Self { results, allow_failure }
}

/// Iterator over all succeeding tests and their names
Expand Down Expand Up @@ -334,14 +324,14 @@ impl TestOutcome {
}

/// Checks if there are any failures and failures are disallowed
pub fn ensure_ok(&self, include_fuzz_test_gas: bool) -> eyre::Result<()> {
pub fn ensure_ok(&self) -> eyre::Result<()> {
if !self.allow_failure {
let failures = self.failures().count();
if failures > 0 {
println!();
println!("Failed tests:");
for (name, result) in self.failures() {
short_test_result(name, include_fuzz_test_gas, result);
short_test_result(name, result);
}
println!();

Expand Down Expand Up @@ -377,7 +367,7 @@ impl TestOutcome {
}
}

fn short_test_result(name: &str, include_fuzz_test_gas: bool, result: &forge::TestResult) {
fn short_test_result(name: &str, result: &forge::TestResult) {
let status = if result.success {
Colour::Green.paint("[PASS]")
} else {
Expand All @@ -397,7 +387,7 @@ fn short_test_result(name: &str, include_fuzz_test_gas: bool, result: &forge::Te
Colour::Red.paint(txt)
};

println!("{} {} {}", status, name, result.kind.gas_used(include_fuzz_test_gas));
println!("{} {} {}", status, name, result.kind.gas_used());
}

/// Runs all the tests
Expand All @@ -407,12 +397,12 @@ fn test(
filter: Filter,
json: bool,
allow_failure: bool,
(include_fuzz_test_gas, gas_reporting, gas_reports): (bool, bool, Vec<String>),
(gas_reporting, gas_reports): (bool, Vec<String>),
) -> eyre::Result<TestOutcome> {
if json {
let results = runner.test(&filter, None)?;
println!("{}", serde_json::to_string(&results)?);
Ok(TestOutcome::new(results, allow_failure, include_fuzz_test_gas))
Ok(TestOutcome::new(results, allow_failure))
} else {
let local_identifier = LocalTraceIdentifier::new(&runner.known_contracts);
let (tx, rx) = channel::<(String, SuiteResult)>();
Expand All @@ -429,7 +419,7 @@ fn test(
println!("Running {} {} for {}", tests.len(), term, contract_name);
}
for (name, result) in &mut tests {
short_test_result(name, include_fuzz_test_gas, result);
short_test_result(name, result);

// We only display logs at level 2 and above
if verbosity >= 2 {
Expand Down Expand Up @@ -492,7 +482,6 @@ fn test(
let block_outcome = TestOutcome::new(
[(contract_name.clone(), suite_result.clone())].into(),
allow_failure,
include_fuzz_test_gas,
);
println!("{}", block_outcome.summary());
results.insert(contract_name, suite_result);
Expand All @@ -505,6 +494,6 @@ fn test(
// reattach the thread
let _ = handle.join();

Ok(TestOutcome::new(results, allow_failure, include_fuzz_test_gas))
Ok(TestOutcome::new(results, allow_failure))
}
}
3 changes: 1 addition & 2 deletions cli/src/forge.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ fn main() -> eyre::Result<()> {
if cmd.build_args().is_watch() {
utils::block_on(watch::watch_test(cmd))?;
} else {
let include_fuzz_test_gas = cmd.include_fuzz_test_gas;
let outcome = cmd.run()?;
outcome.ensure_ok(include_fuzz_test_gas)?;
outcome.ensure_ok()?;
}
}
Subcommands::Bind(cmd) => {
Expand Down
13 changes: 4 additions & 9 deletions forge/src/runner.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl TestResult {
#[derive(Debug, Clone, Eq, PartialEq)]
pub enum TestKindGas {
Standard(u64),
Fuzz { runs: usize, include_fuzz_test_gas: bool, mean: u64, median: u64 },
Fuzz { runs: usize, mean: u64, median: u64 },
}

impl fmt::Display for TestKindGas {
Expand All @@ -92,12 +92,8 @@ impl fmt::Display for TestKindGas {
TestKindGas::Standard(gas) => {
write!(f, "(gas: {})", gas)
}
TestKindGas::Fuzz { runs, include_fuzz_test_gas, mean, median } => {
if *include_fuzz_test_gas {
write!(f, "(runs: {}, μ: {}, ~: {})", runs, mean, median)
} else {
write!(f, "(runs: {})", runs)
}
TestKindGas::Fuzz { runs, mean, median } => {
write!(f, "(runs: {}, μ: {}, ~: {})", runs, mean, median)
}
}
}
Expand Down Expand Up @@ -127,12 +123,11 @@ pub enum TestKind {

impl TestKind {
/// The gas consumed by this test
pub fn gas_used(&self, include_fuzz_test_gas: bool) -> TestKindGas {
pub fn gas_used(&self) -> TestKindGas {
match self {
TestKind::Standard(gas) => TestKindGas::Standard(*gas),
TestKind::Fuzz(fuzzed) => TestKindGas::Fuzz {
runs: fuzzed.cases().len(),
include_fuzz_test_gas,
median: fuzzed.median_gas(false),
mean: fuzzed.mean_gas(false),
},
Expand Down

0 comments on commit 3835e52

Please sign in to comment.