Skip to content

Use nextest to run Miri in CI #9629

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.

Sign up for GitHub

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

Merged
alamb merged 3 commits into from
Apr 1, 2026
Merged

Use nextest to run Miri in CI #9629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e01013
Run miri with nextest
AdamGS Mar 30, 2026
e7ae9a8
Group everything into single miri/nextest group
AdamGS Mar 30, 2026
3b003ec
Install nextest with cargo install
AdamGS Apr 1, 2026
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
11 changes: 5 additions & 6 deletions .github/workflows/miri.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ cargo miri setup
cargo clean

echo "Starting Arrow MIRI run..."
cargo miri test -p arrow-buffer
cargo miri test -p arrow-data --features ffi
cargo miri test -p arrow-schema --features ffi
cargo miri test -p arrow-ord
cargo miri test -p arrow-array
cargo miri test -p arrow-arith
cargo miri nextest run \
Copy link
Copy Markdown
Contributor

@alamb alamb Mar 31, 2026

Choose a reason for hiding this comment

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

TIL: https://nexte.st/docs/integrations/miri/

Copy link
Copy Markdown
Contributor Author

@AdamGS AdamGS Apr 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

nextest is probably the most important non built in tool working in rust for my workflow

Copy link
Copy Markdown
Contributor

@alamb alamb Apr 1, 2026

Choose a reason for hiding this comment

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

sqllogictest is mine 😉

-p arrow-buffer -p arrow-data \
-p arrow-schema -p arrow-ord \
-p arrow-array -p arrow-arith \
--features ffi --no-fail-fast
3 changes: 3 additions & 0 deletions .github/workflows/miri.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ jobs:
rustup toolchain install nightly --component miri
rustup override set nightly
cargo miri setup
- name: Set up nextest
Copy link
Copy Markdown
Contributor

@alamb alamb Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This action scares me security wise (as it runs some bash script)
https://github.com/apache/arrow-rs/actions/runs/23751988211/job/69196496005?pr=9629

I think it would be better (if not ideal) to install nextest explicitly (cargo install cargo-nextest --version 0.9.132 --locked) rather than use this pre-built binary

Copy link
Copy Markdown
Contributor Author

@AdamGS AdamGS Mar 31, 2026

Choose a reason for hiding this comment

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

yeah absolutely, I'll change it.
How would you feel ignoring those long tests and minimizing the first one? I think it might shave the runtime here significantly

Copy link
Copy Markdown
Contributor

@alamb alamb Mar 31, 2026

Choose a reason for hiding this comment

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

How would you feel ignoring those long tests and minimizing the first one? I think it might shave the runtime here significantly

Let's do it

maybe we can make a separate PR to do so (so we can review the changes in isolation)

Copy link
Copy Markdown
Contributor Author

@AdamGS AdamGS Apr 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

SGTM, 3b003ec just installs nextest using cargo install with a locked version, once this merges I'll open up a follow up.

Copy link
Copy Markdown
Contributor

@alamb alamb Apr 1, 2026

Choose a reason for hiding this comment

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

It seems like this adds 3 minutes to the job

https://github.com/apache/arrow-rs/actions/runs/23844918251/job/69509710473?pr=9629

Image

However, we are ahead of where main is

run: |
cargo install cargo-nextest --version 0.9.132 --locked
- name: Run Miri Checks
env:
RUST_BACKTRACE: full
Expand Down
Loading