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.

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

Jump to bottom

Migrate run-make/short-ice to rmake #126036

Merged
merged 2 commits into from
Jun 12, 2024
Merged

Migrate run-make/short-ice to rmake #126036

merged 2 commits into from
Jun 12, 2024

Conversation

Oneirical
Copy link
Contributor

@Oneirical Oneirical commented Jun 5, 2024 •
edited
Loading

Part of #121876 and the associated Google Summer of Code project.

try-job: x86_64-msvc

@rustbot
Copy link
Collaborator

rustbot commented Jun 5, 2024

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 5, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 5, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
Noratrieb
Noratrieb reviewed Jun 5, 2024
View reviewed changes
tests/run-make/short-ice/rmake.rs Outdated
rust_test_log_1.lines().count() < rust_test_log_2.lines().count()
&& count_lines_with(rust_test_log_2, "__rust_begin_short_backtrace")
== count_lines_with(rust_test_log_2, "__rust_end_short_backtrace")
&& count_lines_with(rust_test_log_1, "rustc_query_") + 5 < rustc_query_count_full
&& rustc_query_count_full > 5
Copy link
Member

Choose a reason for hiding this comment

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

just beautiful 🦀

@Oneirical Oneirical force-pushed the the-intelligent-intestor branch 2 times, most recently from d49a7c8 to 824b088 Compare June 6, 2024 14:22
@Oneirical Oneirical marked this pull request as ready for review June 8, 2024 20:00
@rustbot
Copy link
Collaborator

rustbot commented Jun 8, 2024

Some changes occurred in run-make tests.

cc @jieyouxu

@Kobzol
Copy link
Contributor

Kobzol commented Jun 9, 2024

This will need a slight refactoring after #126121, sorry 😇

@Kobzol
Copy link
Contributor

Kobzol commented Jun 9, 2024

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 9, 2024
@jieyouxu jieyouxu mentioned this pull request Jun 9, 2024
Open
@Kobzol Kobzol added the A-run-make Area: port run-make Makefiles to rmake.rs label Jun 9, 2024
@Oneirical
Copy link
Contributor Author

This will need a slight refactoring after #126121, sorry 😇

Not a lot - since this test is merging together stderr and stdout, I don't think I could use the assert-related helper functions. Still got to add a few .stdout_utf8 and such, so that's nice!

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 9, 2024
@Oneirical
Copy link
Contributor Author

@rustbot review
This is probably ready to r+ again, unless we want to investigate the Windows failure more intently.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 11, 2024
jieyouxu
jieyouxu reviewed Jun 11, 2024
View reviewed changes
tests/run-make/short-ice/rmake.rs Outdated
Comment on lines 31 to 35
rust_test_log_1.lines().count() < rust_test_log_2.lines().count()
&& count_lines_with(rust_test_log_2, "__rust_begin_short_backtrace")
== count_lines_with(rust_test_log_2, "__rust_end_short_backtrace")
&& count_lines_with(rust_test_log_1, "rustc_query_") + 5 < rustc_query_count_full
&& rustc_query_count_full > 5
Copy link
Member

Choose a reason for hiding this comment

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

Discussion: should we split this conjunction of multiple clauses into individual asserts, so that if the test does fail we have a chance to know which particular clause failed? I'm personally fine with the test still being //@ ignore-windows, but I would like to know more specificially about which clause causes the test to fail on Windows so we can leave a FIXME.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a very good idea, actually! I will now divide the assert, add a try job, and we can run that to see which one fails, and then restore the ignore.

@jieyouxu
Copy link
Member

@bors delegate+ (so you can run try jobs, but please don't r+ yet until i take another look)

@bors
Copy link
Contributor

bors commented Jun 11, 2024

✌️ @Oneirical, 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

@Oneirical
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Jun 11, 2024

⌛ Trying commit 2e4eff7 with merge 69acf76...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 11, 2024
@bors
Auto merge of rust-lang#126036 - Oneirical:the-intelligent-intestor, …
69acf76 
…r=<try>

Migrate `run-make/short-ice` to `rmake`

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).

try-job: x86_64-msvc
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Jun 11, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 11, 2024
@Oneirical
Copy link
Contributor Author

Failed as expected. The assert_eq fails because of slight differences in how Windows outputs errors.

Ignore restored and reason added.

@Oneirical
Copy link
Contributor Author

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 12, 2024
@jieyouxu
Copy link
Member

Thanks! @bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 12, 2024

📌 Commit 8a6bc13 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 Jun 12, 2024
@compiler-errors compiler-errors mentioned this pull request Jun 12, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 12, 2024
@bors
Auto merge of rust-lang#126345 - compiler-errors:rollup-lue8u92, r=co…
8337ba9 
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125869 (Add `target_env = "p1"` to the `wasm32-wasip1` target)
 - rust-lang#126019 (Add TODO comment to unsafe env modification)
 - rust-lang#126036 (Migrate `run-make/short-ice` to `rmake`)
 - rust-lang#126276 (Detect pub structs never constructed even though they impl pub trait with assoc constants)
 - rust-lang#126282 (Ensure self-contained linker is only enabled on dev/nightly )
 - rust-lang#126317 (Avoid a bunch of booleans in favor of Result<(), ErrorGuaranteed> as that more robustly proves that an error has been emitted)
 - rust-lang#126324 (Adjust LoongArch64 data layouts for LLVM update)
 - rust-lang#126340 (Fix outdated predacates_of.rs comments)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d25227c into rust-lang:master Jun 12, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 12, 2024
@rust-timer
Unrolled build for rust-lang#126036
ddbebaa 
Rollup merge of rust-lang#126036 - Oneirical:the-intelligent-intestor, r=jieyouxu

Migrate `run-make/short-ice` to `rmake`

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).

try-job: x86_64-msvc
@Oneirical Oneirical deleted the the-intelligent-intestor branch June 13, 2024 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kobzol Kobzol Kobzol left review comments

@jieyouxu jieyouxu jieyouxu left review comments

@Noratrieb Noratrieb Noratrieb left review comments

Assignees

@jieyouxu jieyouxu

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
Oxidizing run-make tests
Status: Done
Milestone
1.81.0
Development

Successfully merging this pull request may close these issues.
8 participants
@Oneirical @rustbot @rust-log-analyzer @Kobzol @jieyouxu @bors @matthiaskrgr @Noratrieb