-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
Re-enable a bunch of debuginfo tests. #53154
Re-enable a bunch of debuginfo tests. #53154
Conversation
This comment has been minimized.
This comment has been minimized.
d485ef5
to
4eb52ff
Compare
Seems to work now. r? @kennytm |
@@ -40,14 +39,15 @@ | |||
// gdbr-check:$6 = core::option::Option::None | |||
|
|||
// gdb-command: print os_string | |||
// gdb-check:$7 = "IAMA OS string 😃" | |||
// gdb-check:$7 = "IAMA OS string" |
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.
Is the emoji essential? #42278 (comment) mentioned something about Unicode.
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 don't think it's essential. In my terminal emulator this prints as the emoji, but on CI it seems to print escaped to octals. I did not find a quick way to make print the same everywhere, so I figured that it's better to have a test for the simple case instead of none at all.
@bors r+ rollup |
📌 Commit 4eb52ff has been approved by |
…ginfo-tests, r=kennytm Re-enable a bunch of debuginfo tests. Re-enable some more debuginfo tests that actually seem to work.
…ginfo-tests, r=kennytm Re-enable a bunch of debuginfo tests. Re-enable some more debuginfo tests that actually seem to work.
…ginfo-tests, r=kennytm Re-enable a bunch of debuginfo tests. Re-enable some more debuginfo tests that actually seem to work.
…ginfo-tests, r=kennytm Re-enable a bunch of debuginfo tests. Re-enable some more debuginfo tests that actually seem to work.
This seems to have caused test failures in rollup #53206: see https://ci.appveyor.com/project/rust-lang/rust/build/1.0.8503/job/nw74idt1w1skyomw: Logs
|
@bors rollup- |
⌛ Testing commit 4eb52ff with merge 97e096691801785e730dd1e1dcae26f4dc29c538... |
|
4eb52ff
to
5dd8ed0
Compare
GDB has always been flaky on Windows. Let's not let that prevent us from testing on other platforms. |
@bors r+ rollup |
📌 Commit 5dd8ed0 has been approved by |
…ginfo-tests, r=kennytm Re-enable a bunch of debuginfo tests. Re-enable some more debuginfo tests that actually seem to work.
Adding this to a rollup is risky business |
heh. @bors rollup- r- |
Triage: @kennytm What is left to be done with this PR? |
@Aaronepower The reenabled tests caused #53309 (comment) to fail and must be fixed before approving again. |
ping from triage @kennytm @michaelwoerister what's the update on this? |
I don't really have time to look into this |
Re-enable some more debuginfo tests that actually seem to work.