Skip to content

Commit

Permalink
CI tool usage (#3876)
Browse files Browse the repository at this point in the history
# Objective

- Original objective was to add doc build warning check to the ci local execution
- I somewhat deviated and changed other things...

## Solution

`cargo run -p ci` can now take more parameters:
* `format` - cargo fmt
* `clippy` - clippy
* `compile-fail` - bevy_ecs_compile_fail_tests tests
* `test` - tests but not doc tests and do not build examples
* `doc-test` - doc tests
* `doc-check` - doc build and warnings
* `bench-check` - check that benches build
* `example-check` - check that examples build
* `lints` - group - run lints and format and clippy
* `doc` - group - run doc-test and doc-check
* `compile` - group - run compile-fail and bench-check and example-check
* not providing a parameter will run everything

Ci is using those when possible:
* `build` jobs now don't run doc tests and don't build examples. it makes this job faster, but doc tests and examples are not built for each architecture target
* `ci` job doesn't run the `compile-fail` part but only format and clippy, taking less time
* `check-benches` becomes `check-compiles` and runs the `compile` tasks. It takes longer. I also fixed how it was using cache
* `check-doc` job is now independent and also run the doc tests, so it takes longer. I commented out the deadlinks check as it takes 2.5 minutes (to install) and doesn't work
  • Loading branch information
mockersf committed May 2, 2022
1 parent b863c90 commit 4dbf857
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 39 deletions.
2 changes: 1 addition & 1 deletion .github/bors.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ status = [
"check-unused-dependencies",
"ci",
"miri",
"check-benches",
"check-compiles",
]

use_squash_merge = true
Expand Down
44 changes: 30 additions & 14 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ jobs:
run: sudo apt-get update; sudo apt-get install --no-install-recommends libasound2-dev libudev-dev
if: runner.os == 'linux'
- name: Build & run tests
run: cargo test --workspace
# See tools/ci/src/main.rs for the commands this runs
run: cargo run -p ci -- test
env:
CARGO_INCREMENTAL: 0
RUSTFLAGS: "-C debuginfo=0 -D warnings"
Expand All @@ -68,7 +69,7 @@ jobs:
run: sudo apt-get update; sudo apt-get install --no-install-recommends libasound2-dev libudev-dev libwayland-dev libxkbcommon-dev
- name: CI job
# See tools/ci/src/main.rs for the commands this runs
run: cargo run -p ci -- nonlocal
run: cargo run -p ci -- lints

miri:
runs-on: ubuntu-latest
Expand Down Expand Up @@ -102,7 +103,7 @@ jobs:
# to raw pointer aliasing rules that we should be trying to uphold.
MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-ignore-leaks -Zmiri-tag-raw-pointers

check-benches:
check-compiles:
runs-on: ubuntu-latest
needs: ci
steps:
Expand All @@ -115,15 +116,17 @@ jobs:
~/.cargo/registry/cache/
~/.cargo/git/db/
target/
key: ${{ runner.os }}-cargo-check-benches-${{ hashFiles('**/Cargo.toml') }}
crates/bevy_ecs_compile_fail_tests/target/
key: ${{ runner.os }}-cargo-check-compiles-${{ hashFiles('**/Cargo.toml') }}
- uses: actions-rs/toolchain@v1
with:
toolchain: stable
override: true
- name: Install alsa and udev
run: sudo apt-get update; sudo apt-get install --no-install-recommends libasound2-dev libudev-dev
- name: Check Benches
run: cd benches && cargo check --benches
- name: Check Compile
# See tools/ci/src/main.rs for the commands this runs
run: cargo run -p ci -- compile

build-wasm:
strategy:
Expand Down Expand Up @@ -292,23 +295,36 @@ jobs:

check-doc:
runs-on: ubuntu-latest
needs: check-markdown-links
if: always()
steps:
- uses: actions/checkout@v3
- uses: actions/cache@v2
with:
path: |
~/.cargo/bin/
~/.cargo/registry/index/
~/.cargo/registry/cache/
~/.cargo/git/db/
target/
key: ${{ runner.os }}-check-doc-${{ hashFiles('**/Cargo.toml') }}
- uses: actions-rs/toolchain@v1
with:
toolchain: stable
- name: Install alsa and udev
run: sudo apt-get update; sudo apt-get install --no-install-recommends libasound2-dev libudev-dev libwayland-dev libxkbcommon-dev
if: runner.os == 'linux'
- name: Installs cargo-deadlinks
run: cargo install --force cargo-deadlinks
- name: Build and check doc
run: RUSTDOCFLAGS='-D warnings' cargo doc --workspace --all-features --no-deps --document-private-items
- name: Checks dead links
run: cargo deadlinks --dir target/doc/bevy
continue-on-error: true
# See tools/ci/src/main.rs for the commands this runs
run: cargo run -p ci -- doc
env:
CARGO_INCREMENTAL: 0
RUSTFLAGS: "-C debuginfo=0"
# This currently report a lot of false positives
# Enable it again once it's fixed - https://github.com/bevyengine/bevy/issues/1983
# - name: Installs cargo-deadlinks
# run: cargo install --force cargo-deadlinks
# - name: Checks dead links
# run: cargo deadlinks --dir target/doc/bevy
# continue-on-error: true

check-missing-examples-in-docs:
runs-on: ubuntu-latest
Expand Down
11 changes: 7 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,13 @@ If you're new to Bevy, here's the workflow we use:
1. Fork the `bevyengine/bevy` repository on GitHub. You'll need to create a GitHub account if you don't have one already.
2. Make your changes in a local clone of your fork, typically in its own new branch.
1. Try to split your work into separate commits, each with a distinct purpose. Be particularly mindful of this when responding to reviews so it's easy to see what's changed.
3. To test CI validations locally, run the `cargo run -p ci` command. You can also run sub-commands manually:
1. `cargo fmt --all -- --check` (remove `--check` to let the command fix found problems)
2. `cargo clippy --workspace --all-targets --all-features -- -D warnings -A clippy::type_complexity`
3. `cargo test --all-targets --workspace`
3. To test CI validations locally, run the `cargo run -p ci` command. This will run most checks that happen in CI, but can take some time. You can also run sub-commands to iterate faster depending on what you're contributing:
* `cargo run -p ci -- lints` - to run formatting and clippy
* `cargo run -p ci -- test` - to run tests
* `cargo run -p ci -- doc` - to run doc tests and doc checks
* `cargo run -p ci -- compile` - to check that everything that must compile still does (examples and benches), and that some that shouldn't still don't ([`crates/bevy_ecs_compile_fail_tests`](./crates/bevy_ecs_compile_fail_tests))
* to get more informations on commands available and what is run, check the [tools/ci crate](./tools/ci)

4. When working with Markdown (`.md`) files, Bevy's CI will check markdown files (like this one) using [markdownlint](https://github.com/DavidAnson/markdownlint).
To locally lint your files using the same workflow as our CI:
1. Install [markdownlint-cli](https://github.com/igorshubovych/markdownlint-cli).
Expand Down
1 change: 1 addition & 0 deletions tools/ci/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ license = "MIT OR Apache-2.0"

[dependencies]
xshell = "0.1"
bitflags = "1.3"
95 changes: 75 additions & 20 deletions tools/ci/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,44 +1,99 @@
use xshell::{cmd, pushd};

use bitflags::bitflags;

bitflags! {
struct Check: u32 {
const FORMAT = 0b00000001;
const CLIPPY = 0b00000010;
const COMPILE_FAIL = 0b00000100;
const TEST = 0b00001000;
const DOC_TEST = 0b00010000;
const DOC_CHECK = 0b00100000;
const BENCH_CHECK = 0b01000000;
const EXAMPLE_CHECK = 0b10000000;
}
}

fn main() {
// When run locally, results may differ from actual CI runs triggered by
// .github/workflows/ci.yml
// - Official CI runs latest stable
// - Local runs use whatever the default Rust is locally

// See if any code needs to be formatted
cmd!("cargo fmt --all -- --check")
.run()
.expect("Please run 'cargo fmt --all' to format your code.");
let what_to_run = match std::env::args().nth(1).as_deref() {
Some("format") => Check::FORMAT,
Some("clippy") => Check::CLIPPY,
Some("compile-fail") => Check::COMPILE_FAIL,
Some("test") => Check::TEST,
Some("doc-test") => Check::DOC_TEST,
Some("doc-check") => Check::DOC_CHECK,
Some("bench-check") => Check::BENCH_CHECK,
Some("example-check") => Check::EXAMPLE_CHECK,
Some("lints") => Check::FORMAT | Check::CLIPPY,
Some("doc") => Check::DOC_TEST | Check::DOC_CHECK,
Some("compile") => Check::COMPILE_FAIL | Check::BENCH_CHECK | Check::EXAMPLE_CHECK,
_ => Check::all(),
};

// See if clippy has any complaints.
// - Type complexity must be ignored because we use huge templates for queries
cmd!("cargo clippy --workspace --all-targets --all-features -- -D warnings -A clippy::type_complexity -W clippy::doc_markdown")
if what_to_run.contains(Check::FORMAT) {
// See if any code needs to be formatted
cmd!("cargo fmt --all -- --check")
.run()
.expect("Please run 'cargo fmt --all' to format your code.");
}

if what_to_run.contains(Check::CLIPPY) {
// See if clippy has any complaints.
// - Type complexity must be ignored because we use huge templates for queries
cmd!("cargo clippy --workspace --all-targets --all-features -- -A clippy::type_complexity -W clippy::doc_markdown -D warnings")
.run()
.expect("Please fix clippy errors in output above.");
}

// Run UI tests (they do not get executed with the workspace tests)
// - See crates/bevy_ecs_compile_fail_tests/README.md
{
if what_to_run.contains(Check::COMPILE_FAIL) {
// Run UI tests (they do not get executed with the workspace tests)
// - See crates/bevy_ecs_compile_fail_tests/README.md
let _bevy_ecs_compile_fail_tests = pushd("crates/bevy_ecs_compile_fail_tests")
.expect("Failed to navigate to the 'bevy_ecs_compile_fail_tests' crate");
cmd!("cargo test")
cmd!("cargo test --target-dir ../../target")
.run()
.expect("Compiler errors of the ECS compile fail tests seem to be different than expected! Check locally and compare rust versions.");
}

// These tests are already run on the CI
// Using a double-negative here allows end-users to have a nicer experience
// as we can pass in the extra argument to the CI script
let args: Vec<String> = std::env::args().collect();
if args.get(1) != Some(&"nonlocal".to_string()) {
// Run tests
cmd!("cargo test --workspace")
if what_to_run.contains(Check::TEST) {
// Run tests (except doc tests and without building examples)
cmd!("cargo test --workspace --lib --bins --tests --benches")
.run()
.expect("Please fix failing tests in output above.");
}

if what_to_run.contains(Check::DOC_TEST) {
// Run doc tests
cmd!("cargo test --workspace --doc")
.run()
.expect("Please fix failing doc-tests in output above.");
}

if what_to_run.contains(Check::DOC_CHECK) {
// Check that building docs work and does not emit warnings
std::env::set_var("RUSTDOCFLAGS", "-D warnings");
cmd!("cargo doc --workspace --all-features --no-deps --document-private-items")
.run()
.expect("Please fix doc warnings in output above.");
}

if what_to_run.contains(Check::COMPILE_FAIL) {
// Check that benches are building
let _benches = pushd("benches").expect("Failed to navigate to the 'benches' folder");
cmd!("cargo check --benches --target-dir ../target")
.run()
.expect("Failed to check the benches.");
}

// Run doc tests: these are ignored by `cargo test`
cmd!("cargo test --doc --workspace")
if what_to_run.contains(Check::EXAMPLE_CHECK) {
// Build examples and check they compile
cmd!("cargo check --workspace --examples")
.run()
.expect("Please fix failing doc-tests in output above.");
}
Expand Down

0 comments on commit 4dbf857

Please sign in to comment.