-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add and improve debuginfo tests for Windows #135192
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
CC @wesleywiser |
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.
Do these tests need to be gated by some kind of minimum cdb version?
They should not. I will try to get some old cdb and verify. |
FTR, that is mostly relevant in periods of times where our CI has like a weird mix of 2 cdb versions, e.g. when github gradually rolls out Windows server version bumps. Probably not needed in this case. |
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 @jdupak-ms! I don't see anything here that should require a particularly recent version of cdb to work but if you can test on an older version, that would be great!
If not, let us know and we can merge.
I don't really expect a cdb version problem since we're currently not in a cdb transition period, so let's just send this off to see if this passes full CI. @bors r=@wesleywiser rollup=iffy (debuginfo windows tests) |
OK, sorry for the delay. I had trouble with getting the older cdb to be picked up on my machine. |
Thanks for trying anyway! |
Add and improve debuginfo tests for Windows Adds new test for closures and function pointers. Improves robustness of existing tests by sorting wildcard matched outputs.
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
Ah, you may have to add a
|
Fixed, |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
The failure seems genuine. I added a A cursory glance suggests this is primarily due to pointer-width differences ( |
@bors try |
Add and improve debuginfo tests for Windows Adds new test for closures and function pointers. Improves robustness of existing tests by sorting wildcard matched outputs. try-job: i686-msvc
Thanks a lot (for all your help really). I had an Sorry for taking so many tries. |
No need to be sorry, debuginfo tests are bound to take Several Attempts™. |
FWIW, this is kinda of a limitation of the present debuginfo test infra. Ideally you'd be able to have a revision matrix of sorts (and thus be able to use |
☀️ Try build successful - checks-actions |
@rustbot ready |
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (2ae9916): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (secondary 0.1%)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.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 763.215s -> 763.959s (0.10%) |
Adds new test for closures and function pointers.
Improves robustness of existing tests by sorting wildcard matched outputs.
try-job: i686-msvc