Skip to content

Commit

Permalink
Removed some non-needed dependencies (#2246)
Browse files Browse the repository at this point in the history
<!---
Thank you for contributing to Boa! Please fill out the template below, and remove or add any
information as you feel neccesary.
--->

This Pull Request removes two dependencies that were not really needed, and fixes #2244 by no longer having the package in the dependency tree.

It changes the following:

- The `structopt` dependency in `boa_tester` has been replaced by `clap` v3, the same way as we did in `boa_cli`. This means that we have one less dependency (at least), and that `clap` v2 is only used as a dev-dependency by `criterion` (which will probably be removed in 0.4, as per bheisler/criterion.rs#596).
- The no-longer-updated `num-format` dependency has been removed from `boa_tester`. We were only using it to add comma thousands separator on results, so I added a simple function to do it (not very performant, but it will only be used a few times when showing results).

Looking at this, I noticed a couple of things:

 - The `csv` dependency, used by `criterion` has not been updated in more than a year, and it's using a very old `itoa` dependency. They updated the dependency in the repository in March, but unfortunately, the release is taking some more time than expected, and a tracking issue can be found here: BurntSushi/rust-csv#271
 - `cargo update` fails, because the latest update to `tinystr` in the ICU4x breaks ICU4x 0.6. I have reported this here: unicode-org/icu4x#2428 and their recommendation is for us to use a beta version of the library, but I don't think we should go for that, since this is a semver breakage.
  • Loading branch information
Razican committed Aug 22, 2022
1 parent 861b9a1 commit f0caa3c
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 114 deletions.
90 changes: 3 additions & 87 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion boa_cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ const READLINE_COLOR: Color = Color::Cyan;
// https://docs.rs/structopt/0.3.11/structopt/#type-magic
#[allow(clippy::option_option)]
#[derive(Debug, Parser)]
#[clap(author, about, name = "boa")]
#[clap(author, version, about, name = "boa")]
struct Opt {
/// The JavaScript file(s) to be evaluated.
#[clap(name = "FILE", parse(from_os_str))]
Expand Down
3 changes: 1 addition & 2 deletions boa_tester/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ publish = false
boa_engine = { path = "../boa_engine", features = ["intl"], version = "0.15.0" }
boa_interner = { path = "../boa_interner", version = "0.15.0" }
boa_gc = { path = "../boa_gc", version = "0.15.0" }
structopt = "0.3.26"
clap = { version = "3.2.17", features = ["derive"] }
serde = { version = "1.0.143", features = ["derive"] }
serde_yaml = "0.9.9"
serde_json = "1.0.83"
Expand All @@ -24,7 +24,6 @@ regex = "1.6.0"
once_cell = "1.13.1"
colored = "2.0.0"
fxhash = "0.2.1"
num-format = "0.4.0"
gc = { version = "0.4.1", features = ["derive"] }
rayon = "1.5.3"
anyhow = "1.0.62"
24 changes: 12 additions & 12 deletions boa_tester/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ use self::{
};
use anyhow::{bail, Context};
use bitflags::bitflags;
use clap::Parser;
use colored::Colorize;
use fxhash::{FxHashMap, FxHashSet};
use once_cell::sync::Lazy;
Expand All @@ -81,7 +82,6 @@ use std::{
fs,
path::{Path, PathBuf},
};
use structopt::StructOpt;

/// Structure to allow defining ignored tests, features and files that should
/// be ignored even when reading.
Expand Down Expand Up @@ -193,49 +193,49 @@ static IGNORED: Lazy<Ignored> = Lazy::new(|| {
});

/// Boa test262 tester
#[derive(StructOpt, Debug)]
#[structopt(name = "Boa test262 tester")]
#[derive(Debug, Parser)]
#[clap(author, version, about, name = "Boa test262 tester")]
enum Cli {
/// Run the test suite.
Run {
/// Whether to show verbose output.
#[structopt(short, long, parse(from_occurrences))]
#[clap(short, long, parse(from_occurrences))]
verbose: u8,

/// Path to the Test262 suite.
#[structopt(long, parse(from_os_str), default_value = "./test262")]
#[clap(long, parse(from_os_str), default_value = "./test262")]
test262_path: PathBuf,

/// Which specific test or test suite to run. Should be a path relative to the Test262 directory: e.g. "test/language/types/number"
#[structopt(short, long, parse(from_os_str), default_value = "test")]
#[clap(short, long, parse(from_os_str), default_value = "test")]
suite: PathBuf,

/// Optional output folder for the full results information.
#[structopt(short, long, parse(from_os_str))]
#[clap(short, long, parse(from_os_str))]
output: Option<PathBuf>,

/// Execute tests serially
#[structopt(short, long)]
#[clap(short, long)]
disable_parallelism: bool,
},
Compare {
/// Base results of the suite.
#[structopt(parse(from_os_str))]
#[clap(parse(from_os_str))]
base: PathBuf,

/// New results to compare.
#[structopt(parse(from_os_str))]
#[clap(parse(from_os_str))]
new: PathBuf,

/// Whether to use markdown output
#[structopt(short, long)]
#[clap(short, long)]
markdown: bool,
},
}

/// Program entry point.
fn main() {
match Cli::from_args() {
match Cli::parse() {
Cli::Run {
verbose,
test262_path,
Expand Down
35 changes: 23 additions & 12 deletions boa_tester/src/results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,15 +246,26 @@ pub(crate) fn compare_results(base: &Path, new: &Path, markdown: bool) {
let test_diff = compute_result_diff(Path::new(""), &base_results.results, &new_results.results);

if markdown {
use num_format::{Locale, ToFormattedString};
/// Simple function to add commas as thousands separator for integers.
fn pretty_int(i: isize) -> String {
let mut res = String::new();

for (idx, val) in i.to_string().chars().rev().enumerate() {
if idx != 0 && idx % 3 == 0 {
res.insert(0, ',');
}
res.insert(0, val);
}
res
}

/// Generates a proper diff format, with some bold text if things change.
fn diff_format(diff: isize) -> String {
format!(
"{}{}{}{}",
if diff == 0 { "" } else { "**" },
if diff > 0 { "+" } else { "" },
diff.to_formatted_string(&Locale::en),
pretty_int(diff),
if diff == 0 { "" } else { "**" }
)
}
Expand All @@ -265,32 +276,32 @@ pub(crate) fn compare_results(base: &Path, new: &Path, markdown: bool) {
println!("| :---------: | :----------: | :------: | :--------: |");
println!(
"| Total | {} | {} | {} |",
base_total.to_formatted_string(&Locale::en),
new_total.to_formatted_string(&Locale::en),
pretty_int(base_total),
pretty_int(new_total),
diff_format(total_diff),
);
println!(
"| Passed | {} | {} | {} |",
base_passed.to_formatted_string(&Locale::en),
new_passed.to_formatted_string(&Locale::en),
pretty_int(base_passed),
pretty_int(new_passed),
diff_format(passed_diff),
);
println!(
"| Ignored | {} | {} | {} |",
base_ignored.to_formatted_string(&Locale::en),
new_ignored.to_formatted_string(&Locale::en),
pretty_int(base_ignored),
pretty_int(new_ignored),
diff_format(ignored_diff),
);
println!(
"| Failed | {} | {} | {} |",
base_failed.to_formatted_string(&Locale::en),
new_failed.to_formatted_string(&Locale::en),
pretty_int(base_failed),
pretty_int(new_failed),
diff_format(failed_diff),
);
println!(
"| Panics | {} | {} | {} |",
base_panics.to_formatted_string(&Locale::en),
new_panics.to_formatted_string(&Locale::en),
pretty_int(base_panics),
pretty_int(new_panics),
diff_format(panic_diff),
);
println!(
Expand Down

0 comments on commit f0caa3c

Please sign in to comment.