Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Code Coverage #628

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 34 additions & 18 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,34 +1,50 @@
language: rust
# use trusty for newer openblas

sudo: required
dist: trusty

matrix:
include:
- rust: 1.32.0
env:
- FEATURES='test docs'
- RUSTFLAGS='-D warnings'
env: FEATURES="blas serde-1 docs"
- rust: stable
env:
- FEATURES='test docs'
- RUSTFLAGS='-D warnings'
env: FEATURES="blas serde-1 docs"
- rust: beta
env:
- FEATURES='test docs'
- CHANNEL='beta'
- RUSTFLAGS='-D warnings'
env: FEATURES="blas serde-1 docs"
- rust: nightly
env:
- FEATURES='test docs'
- CHANNEL='nightly'
env: FEATURES="blas serde-1 docs nightly"
allow_failures:
- rust: beta
- rust: nightly

env:
global:
- HOST=x86_64-unknown-linux-gnu
- CARGO_INCREMENTAL=0
- RUSTFLAGS="-D warnings"

cache: cargo

addons:
apt:
update: true
packages:
- libopenblas-dev
- gfortran
# kcov dependencies
- cmake
- g++
- pkg-config
- jq
- libcurl4-openssl-dev
- libelf-dev
- libdw-dev
- binutils-dev
- libiberty-dev


before_script:
- ./ci/before_script.sh

script:
- ./scripts/all-tests.sh "$FEATURES" "$CHANNEL"
- ./ci/script.sh

after_success:
- ./ci/after_success.sh
25 changes: 10 additions & 15 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,25 @@ bench = false
test = true

[dependencies]
itertools = { version = "0.8.0", default-features = false }
matrixmultiply = "0.2.0"
num-complex = "0.2"
num-integer = "0.1.39"
num-traits = "0.2"
num-complex = "0.2"
itertools = { version = "0.8.0", default-features = false }

rayon = { version = "1.0.3", optional = true }

## Optional dependencies
approx = { version = "0.3.2", optional = true }

# Use via the `blas` crate feature!
cblas-sys = { version = "0.1.4", optional = true, default-features = false }
blas-src = { version = "0.2.0", optional = true, default-features = false }

matrixmultiply = { version = "0.2.0" }
# Use via the `serde-1` crate feature!
cblas-sys = { version = "0.1.4", optional = true, default-features = false }
serde = { version = "1.0", optional = true }
rayon = { version = "1.0.3", optional = true }

[dev-dependencies]
approx = "0.3.2"
defmac = "0.2"
itertools = { version = "0.8.0", default-features = false, features = ["use_std"] }
quickcheck = { version = "0.8", default-features = false }
rawpointer = "0.1"
approx = "0.3.2"
itertools = { version = "0.8.0", default-features = false, features = ["use_std"] }

[features]
# Enable blas usage
Expand All @@ -60,9 +56,8 @@ blas = ["cblas-sys", "blas-src"]
# Serde 1.0
serde-1 = ["serde"]

# These features are used for testing
test-blas-openblas-sys = ["blas"]
test = ["test-blas-openblas-sys"]
# Enable nightly-only features
nightly = []

# This feature is used for docs
docs = ["approx", "serde-1", "rayon"]
Expand Down
1 change: 1 addition & 0 deletions benches/bench1.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "nightly")]
#![feature(test)]
#![allow(unused_imports)]
#![allow(
Expand Down
1 change: 1 addition & 0 deletions benches/chunks.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "nightly")]
#![feature(test)]

extern crate test;
Expand Down
1 change: 1 addition & 0 deletions benches/construct.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "nightly")]
#![feature(test)]
#![allow(
clippy::many_single_char_names,
Expand Down
1 change: 1 addition & 0 deletions benches/gemv.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "nightly")]
#![feature(test)]
#![allow(
clippy::many_single_char_names,
Expand Down
1 change: 1 addition & 0 deletions benches/higher-order.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "nightly")]
#![feature(test)]
#![allow(
clippy::many_single_char_names,
Expand Down
1 change: 1 addition & 0 deletions benches/iter.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "nightly")]
#![feature(test)]
#![allow(
clippy::many_single_char_names,
Expand Down
1 change: 1 addition & 0 deletions benches/numeric.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "nightly")]
#![feature(test)]

extern crate test;
Expand Down
1 change: 1 addition & 0 deletions benches/par_rayon.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "nightly")]
#![cfg(feature = "rayon")]
#![feature(test)]

Expand Down
6 changes: 2 additions & 4 deletions build.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
///
/// This build script emits the openblas linking directive if requested
///
//! This build script emits the openblas linking directive if requested

fn main() {
println!("cargo:rerun-if-changed=build.rs");
if cfg!(feature = "test-blas-openblas-sys") {
if cfg!(feature = "blas") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't quite right. The BLAS backend is user-selectable. We shouldn't emit a linking directive for the OpenBLAS backend unless testing ndarray's BLAS support. (This directive should not be emitted just because BLAS support is enabled, because the user may want to use a different backend.)

I think the cleanest solution is to delete build.rs entirely (and the corresponding line in Cargo.toml), then add the following to Cargo.toml:

[dependencies]

...

# This should be used only for the `test-blas-openblas-sys` feature.
openblas-src = { version = "0.6.0", optional = true, default-features = false }

[features]

...

# This feature is used for testing BLAS support with the system OpenBLAS.
test-blas-openblas-sys = ["blas", "blas-src/openblas", "openblas-src", "openblas-src/cblas", "openblas-src/system"]

Then, to test BLAS support using the system's installed OpenBLAS, enable the test-blas-openblas-sys feature. (I've tried running cargo test --features=test-blas-openblas-sys on my computer, and this does work without the "openblas-src/system" feature, but not with that feature. I suspect that this is just an issue with my computer's OpenBLAS installation, and that it'll work fine on CI.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I was misunderstanding how this works (and I'm not sure I fully understand what you would like). My reasoning was that if seems weird to have a feature called test-foo. If anything, should we not have a foo feature and then run these tests of cfg!(test) && cfg!(feature = "foo")?

println!("cargo:rustc-link-lib={}=openblas", "dylib");
}
}
39 changes: 39 additions & 0 deletions ci/after_success.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#!/bin/bash

# Echo all commands before executing them
set -o xtrace
# Forbid any unset variables
set -o nounset
# Exit on any error
set -o errexit


run_kcov() {
sh <(cargo kcov --print-install-kcov-sh)
KCOV_BIN="./$(ls | grep kcov)/build/src/kcov"

# Run kcov on all the test suites
cargo kcov --all --no-default-features --output kcov-no-default-features
cargo kcov --all --features "$FEATURES" --output kcov-features

$KCOV_BIN --merge kcov \
kcov-no-default-features \
kcov-features
}

coverage_codecov() {
if [[ "$TRAVIS_RUST_VERSION" != "stable" ]]; then
return
fi

run_kcov

bash <(curl -s https://codecov.io/bash) -s kcov
echo "Uploaded code coverage to codecov.io"
}

main() {
coverage_codecov
}

main
31 changes: 31 additions & 0 deletions ci/before_script.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/bin/bash

# Echo all commands before executing them
set -o xtrace
# Forbid any unset variables
set -o nounset
# Exit on any error
set -o errexit

# Install clippy and rustfmt
rustup_tools() {
rustup component add clippy rustfmt
}

# Install cargo tools
cargo_tools() {
if [[ "$TRAVIS_RUST_VERSION" != "stable" ]]; then
return
fi
cargo install cargo-update || echo "cargo-update already installed"
cargo install cargo-kcov || echo "cargo-kcov already installed"
# Update cached binaries
cargo install-update -a
}

main() {
rustup_tools
cargo_tools
}

main
34 changes: 34 additions & 0 deletions ci/script.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/bin/bash

# Echo all commands before executing them
set -o xtrace
# Forbid any unset variables
set -o nounset
# Exit on any error
set -o errexit

# Ensure there are no outstanding lints.
check_lints() {
## In the future, it would be good if `|| true` can be removed so that
## clippy warnings abort the build.
cargo clippy --all --features "$FEATURES" || true
}

# Ensure the code is correctly formatted.
check_format() {
cargo fmt --all -- --check
}

# Run the test suite.
check_tests() {
cargo test --all --no-default-features
cargo test --all --features "$FEATURES"
}

main() {
check_lints
check_format
check_tests
}

main
1 change: 1 addition & 0 deletions ndarray-rand/benches/bench.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "nightly")]
#![feature(test)]

extern crate test;
Expand Down
7 changes: 1 addition & 6 deletions numeric-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,4 @@ features = ["small_rng"]
test = false

[features]
test_blas = ["ndarray/blas", "ndarray/test-blas-openblas-sys"]

[profile.dev]
opt-level = 2
[profile.test]
opt-level = 2
test_blas = ["ndarray/blas"]
5 changes: 3 additions & 2 deletions parallel/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ keywords = ["data-structure", "multidimensional", "parallel", "concurrent"]
categories = ["data-structures", "science", "concurrency"]

[dependencies]
rayon = { version = "1.0" }
ndarray = { version = "0.12.0", path = "../" }
rayon = "1.0"
ndarray = { version = "0.12.0", path = "../", features = ["rayon"]}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious; why is the "rayon" feature necessary here?

Note that the ndarray-parallel crate (the stuff in the parallel directory) became obsolete after its functionality was added directly to ndarray behind the "rayon" feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly, it wasn't running the tests correctly without the rayon feature being enabled (but it was a little while ago that I did this).


[dev-dependencies]
approx = "0.3.2"
num_cpus = "1.2"
itertools = { version = "0.8.0", default-features = false }

Expand Down
1 change: 1 addition & 0 deletions parallel/benches/rayon.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "nightly")]
#![feature(test)]

extern crate num_cpus;
Expand Down
6 changes: 3 additions & 3 deletions parallel/src/into_impls.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use ndarray::{Array, ArrayView, ArrayViewMut, Dimension, RcArray};
use ndarray::{ArcArray, Array, ArrayView, ArrayViewMut, Dimension};

use NdarrayIntoParallelIterator;
use Parallel;
Expand All @@ -16,7 +16,7 @@ where
}

// This is allowed: goes through `.view()`
impl<'a, A, D> NdarrayIntoParallelIterator for &'a RcArray<A, D>
impl<'a, A, D> NdarrayIntoParallelIterator for &'a ArcArray<A, D>
where
D: Dimension,
A: Sync,
Expand All @@ -41,7 +41,7 @@ where
}

// This is allowed: goes through `.view_mut()`, which is unique access
impl<'a, A, D> NdarrayIntoParallelIterator for &'a mut RcArray<A, D>
impl<'a, A, D> NdarrayIntoParallelIterator for &'a mut ArcArray<A, D>
where
D: Dimension,
A: Sync + Send + Clone,
Expand Down
4 changes: 3 additions & 1 deletion parallel/tests/azip.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
extern crate approx;
extern crate itertools;
extern crate ndarray;
extern crate ndarray_parallel;

use approx::assert_abs_diff_eq;
use itertools::enumerate;
use ndarray::prelude::*;
use ndarray_parallel::par_azip;
Expand Down Expand Up @@ -37,7 +39,7 @@ fn test_par_azip3() {
*c = a.sin();
});
let res = Array::linspace(0., 3.1, 32).mapv_into(f32::sin);
assert!(res.all_close(&ArrayView::from(&c), 1e-4));
assert_abs_diff_eq!(res, ArrayView::from(&c), epsilon = 1e-4);
}

#[should_panic]
Expand Down
4 changes: 3 additions & 1 deletion parallel/tests/rayon.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
extern crate approx;
extern crate ndarray;
extern crate ndarray_parallel;
extern crate rayon;

use approx::assert_abs_diff_eq;
use ndarray::prelude::*;
use ndarray_parallel::prelude::*;

Expand Down Expand Up @@ -30,7 +32,7 @@ fn test_axis_iter_mut() {
.into_par_iter()
.for_each(|mut v| v.mapv_inplace(|x| x.exp()));
println!("{:?}", a.slice(s![..10, ..5]));
assert!(a.all_close(&b, 0.001));
assert_abs_diff_eq!(a, b, epsilon = 1e-3);
}

#[test]
Expand Down
2 changes: 0 additions & 2 deletions parallel/tests/zip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ extern crate ndarray;
extern crate ndarray_parallel;

use ndarray::prelude::*;
use ndarray_parallel::prelude::*;

use ndarray::Zip;

const M: usize = 1024 * 10;
Expand Down
Loading