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

Restore working debuginfo tests by trimming comments from non-header directive lines #47155

Merged
merged 2 commits into from
Jan 6, 2018
Merged

Restore working debuginfo tests by trimming comments from non-header directive lines #47155

merged 2 commits into from
Jan 6, 2018

Conversation

nerd2
Copy link

@nerd2 nerd2 commented Jan 3, 2018

I noticed when adding a debuginfo test that nothing I did caused the test to fail. Tracing back this seems to have been caused by 3e6c83d which broke parsing of the command/check lines, leaving all tests passing without any checking. This commit provides a basic (although still not very robust) restoration of tests and a should-fail test which checks the parser is running

@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 @alexcrichton (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.

@alexcrichton
Copy link
Member

@bors: r+ p=1

Oh dear, thanks so much for catching this! Let's hope we haven't seen any regressions in the meantime...

cc @kennytm

@bors
Copy link
Contributor

bors commented Jan 3, 2018

📌 Commit 28dd4d9 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jan 3, 2018

⌛ Testing commit 28dd4d9 with merge 655dd55b5e1bd372709f744b577c96f609d91201...

@kennytm kennytm added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 3, 2018
@bors
Copy link
Contributor

bors commented Jan 3, 2018

💔 Test failed - status-travis

@nerd2
Copy link
Author

nerd2 commented Jan 3, 2018

Looks like a few failures on both GDB and LLDB. Do you want them fixed in this commit or marked ignore?

@alexcrichton
Copy link
Member

If you know how to fix them that'd be great, otherwise we can open an issue and ignore them.

@nerd2
Copy link
Author

nerd2 commented Jan 3, 2018

@alexcrichton updated this PR to disable them and created issue 47163 to track. Will update issue with ignored tests if this passes.

@nerd2 nerd2 mentioned this pull request Jan 3, 2018
Open
@alexcrichton
Copy link
Member

@bors: r+

Ok, thanks!

@bors
Copy link
Contributor

bors commented Jan 3, 2018

📌 Commit 3c2c978 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jan 3, 2018

⌛ Testing commit 3c2c9780000169572fa1fb517fe26b0ebadca690 with merge 6664ed9ad4ee7b0093cde3b17794176fdf294092...

@bors
Copy link
Contributor

bors commented Jan 3, 2018

💔 Test failed - status-travis

@alexcrichton
Copy link
Member

A few missing tests?

[01:01:23] failures:
[01:01:23]     [debuginfo-gdb] debuginfo/by-value-non-immediate-argument.rs
[01:01:23]     [debuginfo-gdb] debuginfo/function-arg-initialization.rs
[01:01:23]     [debuginfo-gdb] debuginfo/method-on-enum.rs
[01:01:23]     [debuginfo-gdb] debuginfo/struct-in-enum.rs

@kennytm kennytm 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 4, 2018
@nerd2 nerd2 force-pushed the debuginfo_test_fix branch from 3c2c978 to 970d332 Compare January 4, 2018 10:52
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 4, 2018

📌 Commit 970d332 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jan 4, 2018

⌛ Testing commit 970d33261bd8db922f4631707c0d3ed32e2f06c2 with merge 1330a2f53f069a88c9758f2cb6ad61dea9c1e7fc...

@bors
Copy link
Contributor

bors commented Jan 4, 2018

💔 Test failed - status-travis

@nerd2 nerd2 force-pushed the debuginfo_test_fix branch from 970d332 to f025731 Compare January 4, 2018 18:29
@nerd2
Copy link
Author

nerd2 commented Jan 4, 2018

A lot seems to be broken

@kennytm
Copy link
Member

kennytm commented Jan 4, 2018 •
edited
Loading

@bors r=alexcrichton p=15

@bors
Copy link
Contributor

bors commented Jan 4, 2018

📌 Commit f025731 has been approved by alexchrichton

@bors
Copy link
Contributor

bors commented Jan 4, 2018

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Jan 4, 2018

📌 Commit f025731 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jan 4, 2018

⌛ Testing commit f025731a8c7d1befff81ee5e73e999063ceb1fcf with merge 1e66b606a7491b52f53db72a892bba6dff2c8cae...

@bors
Copy link
Contributor

bors commented Jan 4, 2018

💔 Test failed - status-travis

@kennytm
Copy link
Member

kennytm commented Jan 4, 2018

@bors retry

@bors
Copy link
Contributor

bors commented Jan 4, 2018

⌛ Testing commit f025731a8c7d1befff81ee5e73e999063ceb1fcf with merge 04440cf853e3e16e5b942e38a9e880e93c6b139c...

@bors
Copy link
Contributor

bors commented Jan 5, 2018

💔 Test failed - status-travis

@nerd2 nerd2 force-pushed the debuginfo_test_fix branch from f025731 to 57b4fb1 Compare January 5, 2018 07:31
@kennytm
Copy link
Member

kennytm commented Jan 5, 2018

Why is this file added?

https://github.com/nerd2/rust/blob/57b4fb16a2475a1aee676ad4bc74bb10512ea367/test

@nerd2 nerd2 force-pushed the debuginfo_test_fix branch from 57b4fb1 to 0a24acd Compare January 5, 2018 10:47
@kennytm
Copy link
Member

kennytm commented Jan 5, 2018

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Jan 5, 2018

📌 Commit 0a24acd has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jan 5, 2018

⌛ Testing commit 0a24acd with merge 327698f...

bors added a commit that referenced this pull request Jan 5, 2018
@bors
Auto merge of #47155 - nerd2:debuginfo_test_fix, r=alexcrichton
327698f 
Restore working debuginfo tests by trimming comments from non-header directive lines

I noticed when adding a debuginfo test that nothing I did caused the test to fail. Tracing back this seems to have been caused by 3e6c83d which broke parsing of the command/check lines, leaving all tests passing without any checking. This commit provides a basic (although still not very robust) restoration of tests and a should-fail test which checks the parser is running
@bors
Copy link
Contributor

bors commented Jan 6, 2018

💔 Test failed - status-appveyor

@kennytm
Copy link
Member

kennytm commented Jan 6, 2018

@bors retry #46903 (3 hour timeout)

@bors
Copy link
Contributor

bors commented Jan 6, 2018

⌛ Testing commit 0a24acd with merge fdc6ca4...

bors added a commit that referenced this pull request Jan 6, 2018
@bors
Auto merge of #47155 - nerd2:debuginfo_test_fix, r=alexcrichton
fdc6ca4 
Restore working debuginfo tests by trimming comments from non-header directive lines

I noticed when adding a debuginfo test that nothing I did caused the test to fail. Tracing back this seems to have been caused by 3e6c83d which broke parsing of the command/check lines, leaving all tests passing without any checking. This commit provides a basic (although still not very robust) restoration of tests and a should-fail test which checks the parser is running
@bors
Copy link
Contributor

bors commented Jan 6, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing fdc6ca4 to master...

@bors bors merged commit 0a24acd into rust-lang:master Jan 6, 2018
@cbiffle cbiffle mentioned this pull request Apr 5, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nerd2 @rust-highfive @alexcrichton @bors @kennytm