Skip to content

Commit

Permalink
Use GitHub Actions to test with MSRV + stable + beta + nightly. (#488)
Browse files Browse the repository at this point in the history
* Add basic github action for testing against rust-1.39 + stable + nightly.

* Update MSRV to 1.40

* Use &Path rather than &PathBuf, let panic!() take care of formatting.

* Only run 'fmt' on stable.
  • Loading branch information
lemmih committed Jul 26, 2021
1 parent 9b991e0 commit e228444
Show file tree
Hide file tree
Showing 13 changed files with 88 additions and 35 deletions.
52 changes: 52 additions & 0 deletions .github/workflows/ci.yaml
@@ -0,0 +1,52 @@
on:
push:
branches:
- master
pull_request:
branches:
- master

name: tests

jobs:
ci:
runs-on: ubuntu-latest
strategy:
matrix:
rust:
- stable
- beta
- nightly
- 1.40.0 # MSRV

steps:
- uses: actions/checkout@v2

- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: ${{ matrix.rust }}
override: true
components: rustfmt, clippy

- uses: Swatinem/rust-cache@v1

- uses: actions-rs/cargo@v1
with:
command: build

- uses: actions-rs/cargo@v1
with:
command: test

- uses: actions-rs/cargo@v1
if: ${{ matrix.rust == 'stable' }}
with:
command: fmt
args: --all -- --check

- uses: actions-rs/cargo@v1
if: ${{ matrix.rust != '1.40.0' }} # 1.40 has horrible lints.
with:
command: clippy
args: -- -D warnings
4 changes: 2 additions & 2 deletions README.md
Expand Up @@ -117,9 +117,9 @@ For more details, see the [CONTRIBUTING.md file](https://github.com/bheisler/cri
### Compatibility Policy

Criterion.<span></span>rs supports the last three stable minor releases of Rust. At time of
writing, this means Rust 1.40 or later. Older versions may work, but are not tested or guaranteed.
writing, this means Rust 1.50 or later. Older versions may work, but are not guaranteed.

Currently, the oldest version of Rust believed to work is 1.39. Future versions of Criterion.<span></span>rs may
Currently, the oldest version of Rust believed to work is 1.40. Future versions of Criterion.<span></span>rs may
break support for such old versions, and this will not be considered a breaking change. If you
require Criterion.<span></span>rs to work on old versions of Rust, you will need to stick to a
specific patch version of Criterion.<span></span>rs.
Expand Down
6 changes: 3 additions & 3 deletions src/analysis/mod.rs
Expand Up @@ -53,10 +53,10 @@ pub(crate) fn common<M: Measurement, T: ?Sized>(
&criterion.baseline_directory,
&criterion.output_directory,
) {
panic!(format!(
panic!(
"Baseline '{base}' must exist before comparison is allowed; try --save-baseline {base}",
base=criterion.baseline_directory,
));
);
}
}

Expand Down Expand Up @@ -278,7 +278,7 @@ fn regression(
)
.0;

let point = Slope::fit(&data);
let point = Slope::fit(data);
let (lb, ub) = distribution.confidence_interval(config.confidence_level);
let se = distribution.std_dev(None);

Expand Down
2 changes: 1 addition & 1 deletion src/benchmark_group.rs
Expand Up @@ -345,7 +345,7 @@ impl<'a, M: Measurement> BenchmarkGroup<'a, M> {
func.profile(
&self.criterion.measurement,
&id,
&self.criterion,
self.criterion,
&report_context,
duration,
input,
Expand Down
1 change: 1 addition & 0 deletions src/error.rs
Expand Up @@ -5,6 +5,7 @@ use std::fmt;
use std::io;
use std::path::PathBuf;

#[allow(clippy::enum_variant_names)]
#[derive(Debug)]
pub enum Error {
AccessError {
Expand Down
30 changes: 15 additions & 15 deletions src/html/mod.rs
Expand Up @@ -218,12 +218,12 @@ impl<'a> BenchmarkGroup<'a> {
// numerically. Otherwise sort lexicographically.
if values.iter().all(|os| parse_opt(os).is_some()) {
values.sort_unstable_by(|v1, v2| {
let num1 = parse_opt(&v1);
let num2 = parse_opt(&v2);
let num1 = parse_opt(v1);
let num2 = parse_opt(v2);

num1.partial_cmp(&num2).unwrap_or(Ordering::Less)
});
values.dedup_by_key(|os| parse_opt(&os).unwrap());
values.dedup_by_key(|os| parse_opt(os).unwrap());
} else {
values.sort_unstable();
values.dedup();
Expand Down Expand Up @@ -438,14 +438,14 @@ impl Report for Html {

// If all of the value strings can be parsed into a number, sort/dedupe
// numerically. Otherwise sort lexicographically.
if value_strs.iter().all(|os| try_parse(&*os).is_some()) {
if value_strs.iter().all(|os| try_parse(*os).is_some()) {
value_strs.sort_unstable_by(|v1, v2| {
let num1 = try_parse(&v1);
let num2 = try_parse(&v2);
let num1 = try_parse(v1);
let num2 = try_parse(v2);

num1.partial_cmp(&num2).unwrap_or(Ordering::Less)
});
value_strs.dedup_by_key(|os| try_parse(&os).unwrap());
value_strs.dedup_by_key(|os| try_parse(os).unwrap());
} else {
value_strs.sort_unstable();
value_strs.dedup();
Expand All @@ -455,7 +455,7 @@ impl Report for Html {
let samples_with_function: Vec<_> = data
.iter()
.by_ref()
.filter(|&&(ref id, _)| id.function_id.as_ref() == Some(&function_id))
.filter(|&&(id, _)| id.function_id.as_ref() == Some(function_id))
.collect();

if samples_with_function.len() > 1 {
Expand All @@ -476,7 +476,7 @@ impl Report for Html {
let samples_with_value: Vec<_> = data
.iter()
.by_ref()
.filter(|&&(ref id, _)| id.value_str.as_ref() == Some(&value_str))
.filter(|&&(id, _)| id.value_str.as_ref() == Some(value_str))
.collect();

if samples_with_value.len() > 1 {
Expand All @@ -497,7 +497,7 @@ impl Report for Html {
// First sort the ids/data by value.
// If all of the value strings can be parsed into a number, sort/dedupe
// numerically. Otherwise sort lexicographically.
let all_values_numeric = all_data.iter().all(|(ref id, _)| {
let all_values_numeric = all_data.iter().all(|(id, _)| {
id.value_str
.as_ref()
.map(String::as_str)
Expand Down Expand Up @@ -577,7 +577,7 @@ impl Html {
if !different_mean {
explanation_str = "No change in performance detected.".to_owned();
} else {
let comparison = compare_to_threshold(&mean_est, comp.noise_threshold);
let comparison = compare_to_threshold(mean_est, comp.noise_threshold);
match comparison {
ComparisonResult::Improved => {
explanation_str = "Performance has improved.".to_owned();
Expand Down Expand Up @@ -689,7 +689,7 @@ impl Html {
fs::mkdirp(&both_dir)
});

let comp_data = plot_data.comparison(&comp);
let comp_data = plot_data.comparison(comp);

self.plotter.borrow_mut().pdf(plot_ctx, comp_data);
self.plotter.borrow_mut().pdf(plot_ctx_small, comp_data);
Expand Down Expand Up @@ -767,12 +767,12 @@ impl Html {

self.plotter.borrow_mut().violin(plot_ctx, formatter, data);

let value_types: Vec<_> = data.iter().map(|&&(ref id, _)| id.value_type()).collect();
let value_types: Vec<_> = data.iter().map(|&&(id, _)| id.value_type()).collect();
let mut line_path = None;

if value_types.iter().all(|x| x == &value_types[0]) {
if let Some(value_type) = value_types[0] {
let values: Vec<_> = data.iter().map(|&&(ref id, _)| id.as_number()).collect();
let values: Vec<_> = data.iter().map(|&&(id, _)| id.as_number()).collect();
if values.iter().any(|x| x != &values[0]) {
self.plotter
.borrow_mut()
Expand All @@ -785,7 +785,7 @@ impl Html {
let path_prefix = if full_summary { "../.." } else { "../../.." };
let benchmarks = data
.iter()
.map(|&&(ref id, _)| {
.map(|&&(id, _)| {
IndividualBenchmark::from_id(&report_context.output_directory, path_prefix, id)
})
.collect();
Expand Down
6 changes: 3 additions & 3 deletions src/plot/gnuplot_backend/mod.rs
@@ -1,5 +1,5 @@
use std::iter;
use std::path::PathBuf;
use std::path::Path;
use std::process::Child;

use crate::stats::univariate::Sample;
Expand Down Expand Up @@ -40,9 +40,9 @@ const DARK_BLUE: Color = Color::Rgb(31, 120, 180);
const DARK_ORANGE: Color = Color::Rgb(255, 127, 0);
const DARK_RED: Color = Color::Rgb(227, 26, 28);

fn debug_script(path: &PathBuf, figure: &Figure) {
fn debug_script(path: &Path, figure: &Figure) {
if crate::debug_enabled() {
let mut script_path = path.clone();
let mut script_path = path.to_path_buf();
script_path.set_extension("gnuplot");
info!("Writing gnuplot script to {:?}", script_path);
let result = figure.save(script_path.as_path());
Expand Down
2 changes: 1 addition & 1 deletion src/plot/gnuplot_backend/pdf.rs
Expand Up @@ -33,7 +33,7 @@ pub(crate) fn pdf(
format!("Iterations (x 10^{})", exponent)
};

let (xs, ys) = kde::sweep(&scaled_avg_times, KDE_POINTS, None);
let (xs, ys) = kde::sweep(scaled_avg_times, KDE_POINTS, None);
let (lost, lomt, himt, hist) = avg_times.fences();
let mut fences = [lost, lomt, himt, hist];
let _ = formatter.scale_values(typical, &mut fences);
Expand Down
6 changes: 3 additions & 3 deletions src/plot/gnuplot_backend/summary.rs
Expand Up @@ -83,9 +83,9 @@ pub fn line_comparison(
// This assumes the curves are sorted. It also assumes that the benchmark IDs all have numeric
// values or throughputs and that value is sensible (ie. not a mix of bytes and elements
// or whatnot)
for (key, group) in &all_curves.iter().group_by(|&&&(ref id, _)| &id.function_id) {
for (key, group) in &all_curves.iter().group_by(|&&&(id, _)| &id.function_id) {
let mut tuples: Vec<_> = group
.map(|&&(ref id, ref sample)| {
.map(|&&(id, ref sample)| {
// Unwrap is fine here because it will only fail if the assumptions above are not true
// ie. programmer error.
let x = id.as_number().unwrap();
Expand Down Expand Up @@ -185,7 +185,7 @@ pub fn violin(
positions: tics(),
labels: all_curves
.iter()
.map(|&&(ref id, _)| gnuplot_escape(id.as_title())),
.map(|&&(id, _)| gnuplot_escape(id.as_title())),
})
});

Expand Down
2 changes: 1 addition & 1 deletion src/plot/plotters_backend/pdf.rs
Expand Up @@ -197,7 +197,7 @@ pub(crate) fn pdf(
format!("Iterations (x 10^{})", exponent)
};

let (xs, ys) = kde::sweep(&scaled_avg_times, KDE_POINTS, None);
let (xs, ys) = kde::sweep(scaled_avg_times, KDE_POINTS, None);
let (lost, lomt, himt, hist) = avg_times.fences();
let mut fences = [lost, lomt, himt, hist];
let _ = formatter.scale_values(typical, &mut fences);
Expand Down
6 changes: 3 additions & 3 deletions src/plot/plotters_backend/summary.rs
Expand Up @@ -132,9 +132,9 @@ fn line_comparison_series_data<'a>(
// This assumes the curves are sorted. It also assumes that the benchmark IDs all have numeric
// values or throughputs and that value is sensible (ie. not a mix of bytes and elements
// or whatnot)
for (key, group) in &all_curves.iter().group_by(|&&&(ref id, _)| &id.function_id) {
for (key, group) in &all_curves.iter().group_by(|&&&(id, _)| &id.function_id) {
let mut tuples: Vec<_> = group
.map(|&&(ref id, ref sample)| {
.map(|&&(id, ref sample)| {
// Unwrap is fine here because it will only fail if the assumptions above are not true
// ie. programmer error.
let x = id.as_number().unwrap();
Expand Down Expand Up @@ -164,7 +164,7 @@ pub fn violin(

let mut kdes = all_curves
.iter()
.map(|&&(ref id, ref sample)| {
.map(|&&(id, ref sample)| {
let (x, mut y) = kde::sweep(Sample::new(sample), KDE_POINTS, None);
let y_max = Sample::new(&y).max();
for y in y.iter_mut() {
Expand Down
2 changes: 1 addition & 1 deletion src/report.rs
Expand Up @@ -609,7 +609,7 @@ impl Report for CliReport {
if !different_mean {
explanation_str = "No change in performance detected.".to_owned();
} else {
let comparison = compare_to_threshold(&mean_est, comp.noise_threshold);
let comparison = compare_to_threshold(mean_est, comp.noise_threshold);
match comparison {
ComparisonResult::Improved => {
point_estimate_str = self.green(self.bold(point_estimate_str));
Expand Down
4 changes: 2 additions & 2 deletions src/stats/bivariate/mod.rs
Expand Up @@ -97,12 +97,12 @@ where

/// Returns a view into the `X` data
pub fn x(&self) -> &'a Sample<X> {
Sample::new(&self.0)
Sample::new(self.0)
}

/// Returns a view into the `Y` data
pub fn y(&self) -> &'a Sample<Y> {
Sample::new(&self.1)
Sample::new(self.1)
}
}

Expand Down

0 comments on commit e228444

Please sign in to comment.