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

Add cdb debug info tests for some frequently used data types #85420

Closed

Conversation

nanguye2496
Copy link

This PR add several debug info tests to guarantee that the displays of fixed sized arrays, range types, cell types, threads, locks, and mutexes in CDB are correct.

It also updates CDB tests for slices in pretty-std.rs after string visualization in WinDbg is fixed by this PR: #81898.

@rust-highfive
Copy link
Collaborator

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

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 17, 2021
@rust-log-analyzer

This comment has been minimized.

@nanguye2496
Copy link
Author

@Mark-Simulacrum : Hi Mark, I wonder if these tests should be in their own files or should be added to some existing debug info test files?

@Mark-Simulacrum
Copy link
Member

I'm not sure I follow the question - this PR looks reasonable to me I guess? We generally don't worry too much about whether tests are grouped up into same file or separate files.

@nanguye2496
Copy link
Author

nanguye2496 commented May 18, 2021

I'm not sure I follow the question - this PR looks reasonable to me I guess? We generally don't worry too much about whether tests are grouped up into same file or separate files.

Yes, I meant to add if I should put these tests into existing test files. Thanks for the quick reply. If there is no need for further changes, I will merge my commits into one.

@Mark-Simulacrum
Copy link
Member

Please do, thanks. I expect it'll be a bit before I take a look at this in more detail, so no rush!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants