-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Migrate issue-83112-incr-test-moved-file
, type-mismatch-same-crate-name
and issue-109934-lto-debuginfo
run-make
tests to rmake or ui
#127538
Conversation
This PR modifies cc @jieyouxu |
814fec9
to
2adfa14
Compare
@bors try |
Migrate `issue-83112-incr-test-moved-file`, `type-mismatch-same-crate-name` and `issue-109934-lto-debuginfo` `run-make` tests to rmake or ui Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html). I have noticed that the new UI test `debuginfo-lto-alloc` is outputting artifacts that aren't getting cleaned up because of its `-C incremental`. That might be the justification needed to keep it as a run-make test? Try it on: try-job: test-various
☀️ Try build successful - checks-actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! The tests look reasonable, maybe add a remark in the lto test or not, at your discretion, then r=me if another battery of try-jobs come back green ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have noticed that the new UI test
debuginfo-lto-alloc
is outputting artifacts that aren't getting cleaned up because of its-C incremental
. That might be the justification needed to keep it as a run-make test?
If that's the case, let's keep it as a rmake.rs test for now, can you please leave a remark about your finding in case someone tries to port it to ui tests? (We could try to fix ui tests not cleaning this up in another PR, but I'm also fine with leaving it as a rmake.rs test if it does something that's "more special" than ui tests, i.e. I don't want to shoehorn it into ui tests if rmake.rs is less friction).
@bors delegate+ (r=me if try jobs come back green) |
✌️ @Oneirical, you can now approve this pull request! If @jieyouxu told you to " |
@bors try |
Migrate `issue-83112-incr-test-moved-file`, `type-mismatch-same-crate-name` and `issue-109934-lto-debuginfo` `run-make` tests to rmake or ui Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html). I have noticed that the new UI test `debuginfo-lto-alloc` is outputting artifacts that aren't getting cleaned up because of its `-C incremental`. That might be the justification needed to keep it as a run-make test? Try it on: // try-job: test-various // previously passed try-job: armhf-gnu try-job: aarch64-apple try-job: x86_64-msvc
☀️ Try build successful - checks-actions |
@bors r- |
@Oneirical sorry just to clarify, you said that the lto test has residual artifacts as a UI test, do you think we should keep it as a rmake.rs instead? I would be fine not converting it to a UI test and keeping it as a rmake.rs test if it has special handling that ordinary UI tests do not handle. |
Sorry, I missed your upper review comment! I read it now. When running the UI test locally, some files appear in my file tracker that aren't getting ignored by |
Oh okay cool ty |
☀️ Test successful - checks-actions |
Finished benchmarking commit (e1f45a1): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResults (secondary -0.6%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 703.792s -> 705.402s (0.23%) |
Part of #121876 and the associated Google Summer of Code project.
I have noticed that the new UI test
debuginfo-lto-alloc
is outputting artifacts that aren't getting cleaned up because of its-C incremental
. That might be the justification needed to keep it as a run-make test?Try it on:
// try-job: test-various // previously passed
try-job: armhf-gnu
try-job: aarch64-apple
try-job: x86_64-msvc