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 argument to ci for test threads and build jobs #16145

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

BenjaminBrienen
Copy link
Contributor

Objective

Fixes #16051

Solution

Rework flags into arguments and add an arg for the number of test threads and an arg for the number of build jobs.

cargo run -p ci -- --test-threads 1 --build-jobs 1

Testing

Tested CI locally. Build pipeline should also be a good way to catch mistakes in the command line usage.

@BenjaminBrienen BenjaminBrienen self-assigned this Oct 29, 2024
@BenjaminBrienen BenjaminBrienen added C-Feature A new feature, making something new possible A-Build-System Related to build systems or continuous integration D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 29, 2024
tools/ci/src/args.rs Outdated Show resolved Hide resolved
tools/ci/src/ci.rs Outdated Show resolved Hide resolved
tools/ci/src/ci.rs Show resolved Hide resolved
tools/ci/src/args.rs Show resolved Hide resolved
@hukasu
Copy link

hukasu commented Oct 29, 2024

Also should fix #12207

@BenjaminBrienen
Copy link
Contributor Author

Also should fix #12207

More of a workaround than a fix, I think.

Copy link
Member

@BD103 BD103 left a comment

Choose a reason for hiding this comment

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

Looks good!

@hukasu
Copy link

hukasu commented Jan 5, 2025

I'm not understanding how to use the --jobs arg.

--help gives this Usage: ci [--keep-going] [--test-threads <test-threads>] [--jobs <jobs>] [<command>] [<args>], but using cargo run -p ci -- --jobs 4 doc gives error: Unrecognized option: 'jobs4', and using cargo run -p ci -- --jobs=4 doc gives Unrecognized argument: --jobs=4.

@hukasu
Copy link

hukasu commented Jan 5, 2025

--test-threads worked great, no OOM during bevy_gizmos tests

@hukasu
Copy link

hukasu commented Jan 13, 2025

@BenjaminBrienen does --jobs work for you?

@BenjaminBrienen BenjaminBrienen added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Jan 13, 2025
@BenjaminBrienen
Copy link
Contributor Author

I'll test it when I have time later

@BenjaminBrienen BenjaminBrienen removed the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Jan 14, 2025
@hukasu
Copy link

hukasu commented Jan 15, 2025

pulled d7d808d and now i can pass --jobs but the build is still using more parallelism

cargo run -p ci -- --jobs 4 fails on cargo fmt because it does not take --jobs

cargo run -p ci -- --jobs 4 doc fails with

error: Unrecognized option: 'jobs'
error: doctest failed, to rerun pass `-p benches --doc`
thread 'main' panicked at tools/ci/src/ci.rs:72:13:
One or more CI commands failed:
doc-test: Please fix failing doc tests in output above.

and i saw on the console that it is passing as a string(?)

cargo test --workspace --doc " --jobs 4" -- 

@hukasu
Copy link

hukasu commented Jan 15, 2025

cargo run -p ci -- --jobs 4 bench-check also fails with --jobs being included between quotes

cargo check --benches --target-dir ../target --manifest-path "./benches/Cargo.toml --jobs 4"

@BenjaminBrienen
Copy link
Contributor Author

I'll look at this more later thanks

@BenjaminBrienen BenjaminBrienen added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 15, 2025
@hukasu
Copy link

hukasu commented Jan 15, 2025

cargo run -p ci -- --jobs 4 clippy is quoting

cargo clippy --workspace --all-targets "--all-features --jobs 4" -- -Dwarnings

@BenjaminBrienen
Copy link
Contributor Author

BenjaminBrienen commented Jan 15, 2025

Reading the documentation for cmd!, looks like I'll have to approach this differently. Maybe splatting will work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to build systems or continuous integration C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow passing --jobs to ci to prevent OOM on home machines
4 participants