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

xtask: add coverage command #2067

Merged
merged 3 commits into from Dec 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .cargo/config
@@ -0,0 +1,2 @@
[alias]
xtask = "run --package xtask --"
2 changes: 1 addition & 1 deletion .github/pull_request_template.md
Expand Up @@ -10,6 +10,6 @@ Be aware the CI pipeline will check your pull request for the following:
- Rust lints (`make clippy`)
- Rust formatting (`cargo fmt`)
- Python formatting (`black . --check`. You can install black with `pip install black`)
- Compatibility with all supported Python versions for all examples. This uses `tox`; you can do run it using `make test_py`.
- Compatibility with all supported Python versions for all examples. This uses `tox`; you can do run it using `cargo xtask test-py`.

You can run a similar set of checks as the CI pipeline using `make test`.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Expand Up @@ -213,7 +213,7 @@ jobs:
shell: bash
run: |
python -m pip install -U pip tox
make test_py
cargo xtask test-py
env:
CARGO_TARGET_DIR: ${{ github.workspace }}
TOX_TESTENV_PASSENV: "CARGO_BUILD_TARGET CARGO_TARGET_DIR"
Expand Down
3 changes: 2 additions & 1 deletion Cargo.toml
Expand Up @@ -119,7 +119,8 @@ harness = false
members = [
"pyo3-macros",
"pyo3-macros-backend",
"examples"
"examples",
"xtask"
Copy link
Member

Choose a reason for hiding this comment

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

We don't necessarily need to name this "xtask". I don't have a motivation for changing it. But we could.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I don't personally love the name xtask but I do like that it's a documented "approach" (https://github.com/matklad/cargo-xtask), so without good motivation I don't feel a need to change it.

For example I'd personally enjoy the alias being called cargo run-script. There's nothing stopping us adding a couple extra aliases that all do the same later, if we wanted ;)

]

[package.metadata.docs.rs]
Expand Down
16 changes: 16 additions & 0 deletions Makefile
@@ -1,6 +1,7 @@
.PHONY: test test_py publish clippy lint fmt fmt_py fmt_rust

ALL_ADDITIVE_FEATURES = macros multiple-pymethods num-bigint num-complex hashbrown serde indexmap eyre anyhow
COVERAGE_PACKAGES = --package pyo3 --package pyo3-build-config --package pyo3-macros-backend --package pyo3-macros

list_all_additive_features:
@echo $(ALL_ADDITIVE_FEATURES)
Expand All @@ -25,6 +26,21 @@ fmt_rust:
fmt: fmt_rust fmt_py
@true

coverage:
# cargo llvm-cov clean --workspace
# cargo llvm-cov $(COVERAGE_PACKAGES) --no-report
# cargo llvm-cov $(COVERAGE_PACKAGES) --no-report --features abi3
# cargo llvm-cov $(COVERAGE_PACKAGES) --no-report --features $(ALL_ADDITIVE_FEATURES)
# cargo llvm-cov $(COVERAGE_PACKAGES) --no-report --features abi3 $(ALL_ADDITIVE_FEATURES)
bash -c "\
set -a\
source <(cargo llvm-cov show-env)\
export TOX_TESTENV_PASSENV=*\
make test_py\
"
cargo llvm-cov $(COVERAGE_PACKAGES) --no-run --summary-only


clippy:
cargo clippy --features="$(ALL_ADDITIVE_FEATURES)" --all-targets --workspace -- -Dwarnings
cargo clippy --features="abi3 $(ALL_ADDITIVE_FEATURES)" --all-targets --workspace -- -Dwarnings
Expand Down
11 changes: 11 additions & 0 deletions xtask/Cargo.toml
@@ -0,0 +1,11 @@
[package]
name = "xtask"
version = "0.1.0"
edition = "2018"

[dependencies]
anyhow = "1.0.51"

# Clap 3 requires MSRV 1.54
rustversion = "1.0"
structopt = { version = "0.3", default-features = false }
119 changes: 119 additions & 0 deletions xtask/src/main.rs
@@ -0,0 +1,119 @@
use anyhow::{Context, Result};
use std::{collections::HashMap, process::Command};
use structopt::StructOpt;

#[derive(StructOpt)]
enum Subcommand {
/// Runs `cargo llvm-cov` for the PyO3 codebase.
Coverage,
/// Runs tests in examples/ and pytests/
TestPy,
}

impl Subcommand {
fn execute(self) -> Result<()> {
match self {
Subcommand::Coverage => subcommand_coverage(),
Subcommand::TestPy => run_python_tests(None),
}
}
}

fn main() -> Result<()> {
Subcommand::from_args().execute()
}

/// Runs `cargo llvm-cov` for the PyO3 codebase.
fn subcommand_coverage() -> Result<()> {
run(&mut llvm_cov_command(&["clean", "--workspace"]))?;
run(&mut llvm_cov_command(&["--no-report"]))?;

// FIXME: add various feature combinations using 'full' feature.
// run(&mut llvm_cov_command(&["--no-report"]))?;

// XXX: the following block doesn't work until https://github.com/taiki-e/cargo-llvm-cov/pull/115 is merged
let env = get_coverage_env()?;
run_python_tests(&env)?;
// (after here works with stable llvm-cov)

// TODO: add an argument to make it possible to generate lcov report & use this in CI.
run(&mut llvm_cov_command(&["--no-run", "--summary-only"]))?;
Ok(())
}

fn run(command: &mut Command) -> Result<()> {
println!("running: {}", format_command(command));
command.spawn()?.wait()?;
Ok(())
}

fn llvm_cov_command(args: &[&str]) -> Command {
let mut command = Command::new("cargo");
command.args(&["llvm-cov", "--package=pyo3"]).args(args);
command
}

fn run_python_tests<'a>(
env: impl IntoIterator<Item = (&'a String, &'a String)> + Copy,
) -> Result<()> {
for entry in std::fs::read_dir("pytests")? {
let path = entry?.path();
if path.is_dir() && path.join("tox.ini").exists() {
run(Command::new("tox").arg("-c").arg(path).envs(env))?;
}
}
for entry in std::fs::read_dir("examples")? {
let path = entry?.path();
if path.is_dir() && path.join("tox.ini").exists() {
run(Command::new("tox").arg("-c").arg(path).envs(env))?;
}
}
Ok(())
}

fn get_coverage_env() -> Result<HashMap<String, String>> {
let mut env = HashMap::new();

let output = String::from_utf8(llvm_cov_command(&["show-env"]).output()?.stdout)?;

for line in output.trim().split('\n') {
let (key, value) = split_once(line, '=').context("expected '=' in each output line")?;
env.insert(key.to_owned(), value.trim_matches('"').to_owned());
}

env.insert("TOX_TESTENV_PASSENV".to_owned(), "*".to_owned());
env.insert("RUSTUP_TOOLCHAIN".to_owned(), "nightly".to_owned());

Ok(env)
}

// Replacement for str.split_once() on Rust older than 1.52
#[rustversion::before(1.52)]
fn split_once(s: &str, pat: char) -> Option<(&str, &str)> {
let mut iter = s.splitn(2, pat);
Some((iter.next()?, iter.next()?))
}

#[rustversion::since(1.52)]
fn split_once(s: &str, pat: char) -> Option<(&str, &str)> {
s.split_once(pat)
}

#[rustversion::since(1.57)]
fn format_command(command: &Command) -> String {
let mut buf = String::new();
buf.push('`');
buf.push_str(&command.get_program().to_string_lossy());
for arg in command.get_args() {
buf.push(' ');
buf.push_str(&arg.to_string_lossy());
}
buf.push('`');
buf
}

#[rustversion::before(1.57)]
fn format_command(command: &Command) -> String {
// Debug impl isn't as nice as the above, but will do on < 1.57
format!("{:?}", command)
}