Skip to content

modifed y.sh to allow for running cargo tests. - #669

Merged
antoyo merged 2 commits into
rust-lang:masterfrom
FractalFir:y_cargo_tests
May 28, 2025
Merged

modifed y.sh to allow for running cargo tests.#669
antoyo merged 2 commits into
rust-lang:masterfrom
FractalFir:y_cargo_tests

Conversation

@FractalFir

Copy link
Copy Markdown
Contributor

Added a new option to ./y.sh test: ./y.sh test --cargo-tests.

When this option is selected, y.sh will build & test minicore(it is a dependency). After that, it will simply call cargo test with the selected environment flags.

For now ./y.sh test --cargo-tests ignores RUSTFLAGS.

Normally, RUSTFLAGS contain the -Zcodegen-backend option, which would make cg_gcc rebuild itself. This is undesirable, and ignoring RUSTFLAGS altogether was simply the easiest option.

In the future, we maybe could filter out -Zcodegen-backend, and keep the rest of the flags intact.

@antoyo antoyo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good job!

Could you please update the doc and the CI to use this new command instead of cargo test?

@antoyo antoyo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like how you add a lot of comments.
Thanks!

Comment thread build_system/src/test.rs Outdated
Comment thread build_system/src/test.rs

@antoyo antoyo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good!

Comment thread build_system/src/test.rs
@FractalFir
FractalFir force-pushed the y_cargo_tests branch 3 times, most recently from c80276e to 9da753c Compare May 25, 2025 21:46
Comment thread build_system/src/test.rs
@FractalFir
FractalFir force-pushed the y_cargo_tests branch 6 times, most recently from 8a8bf33 to 32fa765 Compare May 27, 2025 22:14
@FractalFir

Copy link
Copy Markdown
Contributor Author

Should be good to merge now.

Comment thread CONTRIBUTING.md Outdated

@antoyo antoyo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A small nitpick:

Comment thread build_system/src/test.rs
@FractalFir

Copy link
Copy Markdown
Contributor Author

Should be good for review now.

@antoyo antoyo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very nice work.
One last thing that I missed.

Comment thread .github/workflows/ci.yml
@FractalFir

Copy link
Copy Markdown
Contributor Author

Should be OK to merge now.

I split the enviroment variable changes into a separate commit, just in case it turns out something still needed that.

@antoyo
antoyo merged commit 706905b into rust-lang:master May 28, 2025
@antoyo

antoyo commented May 28, 2025

Copy link
Copy Markdown
Contributor

Thanks a lot!
This is a very nice improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants