Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix blessing of rmake tests #129040

Merged
merged 2 commits into from
Aug 13, 2024
Merged

Fix blessing of rmake tests #129040

merged 2 commits into from
Aug 13, 2024

Conversation

Zalathar
Copy link
Contributor

Fixes #129038.

When running in --bless mode, we now set the value of RUSTC_BLESS_TEST to the current test's source directory. This allows the diff helper in run_make_support to find the original snapshot file in the source directory and bless that, instead of unhelpfully blessing the temporary copy in build.

r? @jieyouxu

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2024
@rustbot
Copy link
Collaborator

rustbot commented Aug 13, 2024

The run-make-support library was changed

cc @jieyouxu

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Aug 13, 2024
@Zalathar
Copy link
Contributor Author

I thought about using a different variable to communicate the path, but RUSTC_BLESS_TEST is right there, and its value isn't being used for any other purpose anyway, so this avoids adding an extra moving piece.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@jieyouxu
Copy link
Member

I'll r+ after PR CI is green

@jieyouxu
Copy link
Member

Actually I can just let you r=me after PR CI is green

@bors delegate+

@bors
Copy link
Contributor

bors commented Aug 13, 2024

✌️ @Zalathar, you can now approve this pull request!

If @jieyouxu told you to "r=me" after making some further change, please make that change, then do @bors r=@jieyouxu

@jieyouxu
Copy link
Member

Ideally we would want some kind of self-test, but I don't think we want to muck about in actual sources directories.

@Zalathar
Copy link
Contributor Author

Yeah, automatically testing this seems disproportionately hard, so for this PR I didn't bother.

@jieyouxu
Copy link
Member

Yeah, feel free to r=me, I think we can just YOLO it (it was YOLO'd before anyway, so...)

@Zalathar
Copy link
Contributor Author

CI is green.

@bors r=jieyouxu

@bors
Copy link
Contributor

bors commented Aug 13, 2024

📌 Commit cc58cf6 has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 13, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#128643 (Refactor `powerpc64` call ABI handling)
 - rust-lang#128655 (std: refactor UNIX random data generation)
 - rust-lang#128745 (Remove unused lifetime parameter from spawn_unchecked)
 - rust-lang#128841 (bootstrap: don't use rustflags for `--rustc-args`)
 - rust-lang#128983 (Slightly refactor `TargetSelection` in bootstrap)
 - rust-lang#129026 (CFI: Move CFI ui tests to cfi directory)
 - rust-lang#129040 (Fix blessing of rmake tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 42f70c2 into rust-lang:master Aug 13, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 13, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 13, 2024
Rollup merge of rust-lang#129040 - Zalathar:bless-rmake, r=jieyouxu

Fix blessing of rmake tests

Fixes rust-lang#129038.

When running in `--bless` mode, we now set the value of `RUSTC_BLESS_TEST` to the current test's source directory. This allows the diff helper in `run_make_support` to find the original snapshot file in the source directory and bless that, instead of unhelpfully blessing the temporary copy in `build`.

r? `@jieyouxu`
@Zalathar Zalathar deleted the bless-rmake branch August 13, 2024 22:48
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Blessing rmake.rs tests silently does nothing
4 participants