Skip to content

Commit

Permalink
Stability: Add CI to check, test and run safety checks (CeleritasCele…
Browse files Browse the repository at this point in the history
…ry#35)

* Add initial files and document potential improvements

* Rename branches to `master`

Checked with the owner whether he has a preference on `master` compared
to `main`, as we see a general trend of Github repos choosing the later.

* Remove nostd workflow

We don't really support `nostd` enviroments and in all honesty, I don't
see how this project would be used on those.

* Add MIRIFLAGS as specified on README.org

* Document scheduled removal and replace master

Scheduled is a job that runs daily whenever there's a merge on nigthly,
with the hypothesis that it's possible that nightly builds could break
our crate. Since we are not using nightly (just for CI), I propose we
iterate on it. If we feel our CI breaks because of nightly builds, we
can add it back.

For sure we could consider adding it if we ever move our toolchain to nightly.

* Run fmt --check on the stable toolchain

* Enforce nightly on cargo doc

By having the rust toolchain point to beta, we are failing on some of
the CI jobs.

* Deleting rust-toolchain.toml

Having `info: syncing channel updates for
'beta-x86_64-unknown-linux-gnu'` as the toolchain on the repo
is actually breaking CI. Let's see other crates and see what
they are doing.

- serde: no rust-toolchain.toml
- once_cell: no rust-toolchain.toml
- axum: no rust-toolchain.toml

My hypothesis for that is that you don't really need a
rust-toolchain.toml checked out on the repo, but rather we should
ignore it. If you want to use a rust toolchain great, so you avoid
having to run cargo with =+beta=, but no need to have it on version control.

* WIP: Install the pango library before runnning clippy

* Install the gtk-3.0 system library

* Add the system libraries to check and msrv

* Install dependencies on satety and test workflows

* Remove loom and document possible improvements

* Remove ubuntu system on macos and windows

* Add codecov.io to possible improvements

* Test improvement with directed-minimal-versions

* Revert "Test improvement with directed-minimal-versions"

This reverts commit 60d38ff.

* Fix sanitizers not finding any library code

We don't really own any, we are a binary to run Emacs by loading its
elisp files, providing the low level functionality that emacs can't
implement in elisp.

* Remove windows-latest from os matrix

We don't currently support Windows (at least as we see from CI we are
panicking), but definitely something to consider for the future. Added
improvement to DOCS.md.

* Add improvement around doctests on binary crates
  • Loading branch information
Qkessler authored Dec 2, 2023
1 parent dc9b411 commit 66bdeb0
Show file tree
Hide file tree
Showing 7 changed files with 406 additions and 2 deletions.
60 changes: 60 additions & 0 deletions .github/DOCS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Github config and workflows

In this folder there is configuration for codecoverage, dependabot, and ci
workflows that check the library more deeply than the default configurations. This folder was merged <https://github.com/jonhoo/rust-ci-conf/> which provides a reasonably sensible base for writing your own ci on.

An overview of the files in this project is available at:
<https://www.youtube.com/watch?v=xUH-4y92jPg&t=491s>

## Potential additions

Taken from the template above, there are different workflows that wouldn't make sense for the `rune` project. I'll document them here in case we want to change our minds in the future and end up using them.

### cargo-hack
> `cargo-hack` checks combinations of feature flags to ensure that features are all additive which is required for feature unification.
[source](https://github.com/jonhoo/rust-ci-conf/blob/main/.github/workflows/check.yml#L77)

As we don't currently have any meaningful feature gates.

### scheduled
> Run scheduled (rolling) jobs on a nightly basis, as your crate may break independently of any given PR. E.g., updates to rust nightly and updates to this crates dependencies. See check.yml for information about how the concurrency cancelation and workflow triggering works
[source](https://github.com/jonhoo/rust-ci-conf/blob/main/.github/workflows/scheduled.yml#L1)

As we don't run on nightly, we don't really need to check whether nightly breaks our builds. A potential argument for adding this could be that we are running nightly for the CI, and we could break the CI from nightly builds. I propose revisiting adding the `scheduled.yml` linked above if that's the case.

### loom
> Loom is a testing tool for concurrent Rust code. It runs a test many times, permuting the possible concurrent executions of that test under the C11 memory model. It uses state reduction techniques to avoid combinatorial explosion.
[source](https://crates.io/crates/loom)

Loom is great and it's backed by Tokio, but it would mean a bigger investment in making the threads we use be loom specific. Definitely something to iterate on, as we get onto Async Emacs.

### codecov.io
> Enhance Your Testing the Codecov Way: Codecov is the all-in-one code coverage reporting solution for any test suite — giving developers actionable insights to deploy reliable code with confidence.
[source](https://about.codecov.io), [repo of Github Action](https://github.com/codecov/codecov-action)

If we want to add a badge or have nice reports on coverage, we could investigate CodeCov. It requires a token that should be added to the secrets of the repository: `CODECOV_TOKEN`.

### Windows tests
```yaml
matrix:
os: [macos-latest] # REVIEW: We could add windows-latest here.
```
As of today, we don't support Windows, or at least the tests are failing. We can iterate on this and if we feel Windows should be supported in the near feature, we can add `windows-latest` to run CI on it.

### Doctests on --bin crates

As suggested by [issue](https://github.com/rust-lang/rust/issues/50784), doctests won't currently run on Binary creates. I don't see anything particularly wrong with running doctests on binary crates. Some of the argumenst include that "Why would a binary be documented?". Documentation is not only useful for consumers of the library, but rather also internal developers that are maintaining it, looking for better understanding of the binary crate. It's key that developers onboarding into the project have the material to have a swift ramp-up process, to move the project forward.

Once the issue above is stabilized, we can add the following to the test.yml fire:

```yaml
# https://github.com/rust-lang/cargo/issues/6669
- name: cargo test --doc
run: cargo test --locked --all-features --doc
```

21 changes: 21 additions & 0 deletions .github/codecov.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# ref: https://docs.codecov.com/docs/codecovyml-reference
coverage:
# Hold ourselves to a high bar
range: 85..100
round: down
precision: 1
status:
# ref: https://docs.codecov.com/docs/commit-status
project:
default:
# Avoid false negatives
threshold: 1%

# Test files aren't important for coverage
ignore:
- "tests"

# Make comments less noisy
comment:
layout: "files"
require_changes: true
19 changes: 19 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
version: 2
updates:
- package-ecosystem: github-actions
directory: /
schedule:
interval: daily
- package-ecosystem: cargo
directory: /
schedule:
interval: daily
ignore:
- dependency-name: "*"
# patch and minor updates don't matter for libraries as consumers of this library build
# with their own lockfile, rather than the version specified in this library's lockfile
# remove this ignore rule if your package has binaries to ensure that the binaries are
# built with the exact set of dependencies and those are up to date.
update-types:
- "version-update:semver-patch"
- "version-update:semver-minor"
101 changes: 101 additions & 0 deletions .github/workflows/check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# This workflow runs whenever a PR is opened or updated, or a commit is pushed to main. It runs
# several checks:
# - fmt: checks that the code is formatted according to rustfmt
# - clippy: checks that the code does not contain any clippy warnings
# - doc: checks that the code can be documented without errors
# - hack: check combinations of feature flags
# - msrv: check that the msrv specified in the crate is correct
permissions:
contents: read
# This configuration allows maintainers of this repo to create a branch and pull request based on
# the new branch. Restricting the push trigger to the main branch ensures that the PR only gets
# built once.
on:
push:
branches: [master]
pull_request:
# If new code is pushed to a PR branch, then cancel in progress workflows for that PR. Ensures that
# we don't waste CI time, and returns results quicker https://github.com/jonhoo/rust-ci-conf/pull/5
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true
name: check
jobs:
fmt:
runs-on: ubuntu-latest
name: stable / fmt
steps:
- uses: actions/checkout@v4
with:
submodules: true
- name: Install stable
uses: dtolnay/rust-toolchain@stable
with:
components: rustfmt
- name: cargo +stable fmt --check
run: cargo +stable fmt --check
clippy:
runs-on: ubuntu-latest
name: ${{ matrix.toolchain }} / clippy
permissions:
contents: read
checks: write
strategy:
fail-fast: false
matrix:
# Get early warning of new lints which are regularly introduced in beta channels.
toolchain: [stable, beta]
steps:
- uses: actions/checkout@v4
with:
submodules: true
- name: Install the required system libraries
run: sudo apt-get -y install libpango1.0-dev libgtk-3-dev
- name: Install ${{ matrix.toolchain }}
uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.toolchain }}
components: clippy
- name: cargo clippy
uses: actions-rs/clippy-check@v1
with:
token: ${{ secrets.GITHUB_TOKEN }}
doc:
# run docs generation on nightly rather than stable. This enables features like
# https://doc.rust-lang.org/beta/unstable-book/language-features/doc-cfg.html which allows an
# API be documented as only available in some specific platforms.
runs-on: ubuntu-latest
name: nightly / doc
steps:
- uses: actions/checkout@v4
with:
submodules: true
- name: Install the required system libraries
run: sudo apt-get -y install libpango1.0-dev libgtk-3-dev
- name: Install nightly
uses: dtolnay/rust-toolchain@nightly
- name: cargo doc
run: cargo +nightly doc --no-deps --all-features
env:
RUSTDOCFLAGS: --cfg docsrs
msrv:
# check that we can build using the minimal rust version that is specified by this crate
runs-on: ubuntu-latest
# we use a matrix here just because env can't be used in job names
# https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
strategy:
matrix:
msrv: ["1.70.0"] # REVIEW: 2021 edition requires 1.56
name: ubuntu / ${{ matrix.msrv }}
steps:
- uses: actions/checkout@v4
with:
submodules: true
- name: Install the required system libraries
run: sudo apt-get -y install libpango1.0-dev libgtk-3-dev
- name: Install ${{ matrix.msrv }}
uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.msrv }}
- name: cargo +${{ matrix.msrv }} check
run: cargo check
73 changes: 73 additions & 0 deletions .github/workflows/safety.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# This workflow runs checks for unsafe code. In crates that don't have any unsafe code, this can be
# removed. Runs:
# - miri - detects undefined behavior and memory leaks
# - address santizer - detects memory errors
# - leak sanitizer - detects memory leaks
# See check.yml for information about how the concurrency cancelation and workflow triggering works
permissions:
contents: read
on:
push:
branches: [master]
pull_request:
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true
name: safety
jobs:
sanitizers:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: true
- name: Install the required system libraries
run: sudo apt-get -y install libpango1.0-dev libgtk-3-dev
- name: Install nightly
uses: dtolnay/rust-toolchain@nightly
- run: |
# to get the symbolizer for debug symbol resolution
sudo apt install llvm
# to fix buggy leak analyzer:
# https://github.com/japaric/rust-san#unrealiable-leaksanitizer
# ensure there's a profile.dev section
if ! grep -qE '^[ \t]*[profile.dev]' Cargo.toml; then
echo >> Cargo.toml
echo '[profile.dev]' >> Cargo.toml
fi
# remove pre-existing opt-levels in profile.dev
sed -i '/^\s*\[profile.dev\]/,/^\s*\[/ {/^\s*opt-level/d}' Cargo.toml
# now set opt-level to 1
sed -i '/^\s*\[profile.dev\]/a opt-level = 1' Cargo.toml
cat Cargo.toml
name: Enable debug symbols
- name: cargo test -Zsanitizer=address
run: cargo test --all-features --target x86_64-unknown-linux-gnu
env:
ASAN_OPTIONS: "detect_odr_violation=0:detect_leaks=0"
RUSTFLAGS: "-Z sanitizer=address"
- name: cargo test -Zsanitizer=leak
if: always()
run: cargo test --all-features --target x86_64-unknown-linux-gnu
env:
LSAN_OPTIONS: "suppressions=lsan-suppressions.txt"
RUSTFLAGS: "-Z sanitizer=leak"
miri:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: true
- name: Install the required system libraries
run: sudo apt-get -y install libpango1.0-dev libgtk-3-dev
- run: |
echo "NIGHTLY=nightly-$(curl -s https://rust-lang.github.io/rustup-components-history/x86_64-unknown-linux-gnu/miri)" >> $GITHUB_ENV
- name: Install ${{ env.NIGHTLY }}
uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ env.NIGHTLY }}
components: miri
- name: cargo miri test
run: cargo miri test
env:
MIRIFLAGS: "-Zmiri-strict-provenance"
Loading

0 comments on commit 66bdeb0

Please sign in to comment.