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

tests: re-enable pretty-std-collections on macOS #115128

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

davidtwco
Copy link
Member

Fixes #78665.

I made some small modifications to this test so that it would pass for me locally (though I was only able to test using lldb without built-in Rust support, but that seems to be the mode in which it would fail). I ran it a few hundred times with stage one and stage two to see if I could re-produce the spurious failures that were being reported in #78665 and couldn't. From the discussion in #78665, it seemed like this was related to Xcode versions and could be reproduced locally fairly easily. It's been a couple years since this was disabled so a lot has changed. If this starts failing spuriously again then we can disable it and I can look into that.

r? @wesleywiser (discussed in wg-debugging's triage meeting)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 23, 2023
@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 Aug 24, 2023
@davidtwco davidtwco force-pushed the re-enable-debuginfo-test branch 2 times, most recently from 29bac6b to cc0fa87 Compare August 25, 2023 09:32
@wesleywiser
Copy link
Member

Nice! Thanks for looking into the lldb behavior change

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 29, 2023

📌 Commit cc0fa87bd4df34e0294783460c5537f1d1b371ff has been approved by wesleywiser

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 29, 2023
@bors
Copy link
Collaborator

bors commented Aug 29, 2023

⌛ Testing commit cc0fa87bd4df34e0294783460c5537f1d1b371ff with merge 72dcbc69c9c86e26700420b72ac5aa069801bfdf...

@bors
Copy link
Collaborator

bors commented Aug 29, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 29, 2023
Newer lldb versions disable printing of persistent results by default,
but lots of rustc debuginfo tests rely on these being printed, so
re-enable this by defining an alias as suggested by the patch which
disabled persistent result printing in lldb.

Signed-off-by: David Wood <david@davidtw.co>
Signed-off-by: David Wood <david@davidtw.co>
@davidtwco davidtwco force-pushed the re-enable-debuginfo-test branch from cc0fa87 to 3bd5dc3 Compare August 30, 2023 12:13
@davidtwco
Copy link
Member Author

So I had originally fixed the failures that happened with new lldb versions by aliasing print to dwim-print --persistent-result on -- (lldb changed print from expr -- to dwim-print -- in a recent version, which had persistent results off by default). Instead I've just changed it back to expr -- which hopefully will work in the older version we have on CI (otherwise I'll condition this alias on lldb version), and it also fixes the issue I was experiencing in llvm/llvm-project#65076.

@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 12, 2023

📌 Commit 3bd5dc3 has been approved by wesleywiser

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 Sep 12, 2023
@bors
Copy link
Collaborator

bors commented Sep 12, 2023

⌛ Testing commit 3bd5dc3 with merge 960a5ed...

@bors
Copy link
Collaborator

bors commented Sep 12, 2023

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing 960a5ed to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 12, 2023
@bors bors merged commit 960a5ed into rust-lang:master Sep 12, 2023
@rustbot rustbot added this to the 1.74.0 milestone Sep 12, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (960a5ed): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.7% [-0.7%, -0.7%] 1
Improvements ✅
(secondary)
-3.3% [-5.2%, -1.8%] 6
All ❌✅ (primary) -0.7% [-0.7%, -0.7%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 632.565s -> 631.022s (-0.24%)
Artifact size: 317.90 MiB -> 317.92 MiB (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. 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
None yet
Development

Successfully merging this pull request may close these issues.

debuginfo/pretty-std-collections.rs test sometimes fails on macOS
5 participants