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

debuginfo: Preparations for LLDB autotests #13726

Closed

Conversation

michaelwoerister
Copy link
Member

This pull request contains preparations for adding LLDB autotests:

  • the debuginfo tests are split into debuginfo-gdb and debuginfo-lldb
    • the compiletest tool is updated to support the debuginfo-lldb mode
    • tests.mk is modified to provide debuginfo-gdb and debuginfo-lldb make targets
    • GDB test cases are moved from src/test/debug-info to src/test/debuginfo-gdb
  • configure will now look for LLDB and set the appropriate CFG variables
  • the lldb_batchmode.py script is added to src/etc. It emulates GDB's batch mode

The LLDB autotests themselves are not part of this PR. Those will probable require some manual work on the test bots to make them work for the first time. Better to get these unproblematic preliminaries out of the way in a separate step.

@alexcrichton
Copy link
Member

I'm curious if we could take a leaf out of LLVM's book, and use the same test, but with different directives for the test. It seems like we'd only one set of test cases, and optimally have GDB and LLDB test the same set of cases (with opt outs for when one is broken).

Something along the lines of:

// gdb-debugger: <some gdb comand>
// gdb-check: <check some gdb output>
// lldb-debugger: <some lldb comand>
// lldb-check: <check some lldb output>

fn common_test_case() {
}

Would that work out? (it may not for other reasons I don't know)

@brson
Copy link
Contributor

brson commented Apr 25, 2014

I think I agree with @alexcrichton regarding debuginfo and debuginfo-gdb. The scheme with unique tests for each debugger sounds troublesome to maintain.

@michaelwoerister
Copy link
Member Author

I agree, it would be better to have just one shared test file for both debuggers. I actually started out implementing it that way but gave up on the idea a day into it, when I discovered that I had already modified way too many parts of the compiletest tool and saw that I would have had to modify even more if I continued following my design. However, that was nearly two months ago and in the meantime the testing methods for LLDB are not in flux anymore and I have a better grasp on the build system. I think that it shouldn't actually be too hard to have a single set of test files for both GDB and LLDB, much like the run-pass test files are re-used for the pretty printing tests.

The new plan would be to have one src/test/debuginfo directory but two separate compiletest modes. Debugger command parsing is then relegated to the run_{GDB|LLDB}_test() functions. It's not the prettiest approach, but trying to refactor compiletest's monolithic design into something more modular was what was causing most of the trouble the first time around.

I'll sketch the new solution next week. If I'm not missing something here, it should be a matter of about a day to implement it and port the tests.

@brson
Copy link
Contributor

brson commented Apr 25, 2014

Sounds good. If that's the only problem here and you have a plan to address it, perhaps we can merge. @alexcrichton any other issues here?

@alexcrichton
Copy link
Member

That sounds good to me! And this otherwise looks great.

We may want to hold off on merging though because it looks like good chunks are going to get reverted if we leave it pretty much as-is and add some extra support for differentiating lldb/gdb.

@michaelwoerister
Copy link
Member Author

Yes, I don't think that merging would be advisable now. Especially the 90 test-file renamings won't happen after all. Let's put this on hold until I had a chance of making the suggested changes (i.e. next Wednesday). You can also close this particular PR, if you don't want to have it hanging around this long.

@michaelwoerister
Copy link
Member Author

OK, I've updated this. Now there will only be one source file per test, which can contain separate commands for GDB and LLDB. There are two compiletest modes running in two different build directories. I've also consistently renamed debug-info to debuginfo everywhere I saw it.

The PR should be ready for another inspection now.

@@ -0,0 +1,170 @@
# Copyright 2014 The Rust Project Developers. See the COPYRIGHT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment at the top of this file explaining what it's used for?

@alexcrichton
Copy link
Member

This is fantastic, nice work!

I only have on minor comment, so I'm going to r+ this and any touch-ups can happen in later PRs

@michaelwoerister
Copy link
Member Author

I'll add the comment in the coming PR.
Is this a real build error?

@jdm
Copy link
Contributor

jdm commented May 1, 2014

For the record:

config.mk:38: *** missing separator.  Stop.
program finished with exit code 2
elapsedTime=0.009072

@michaelwoerister
Copy link
Member Author

It seems that LLDB only implements the --python-path commandline option since about a year ago. This causes the configure script to create an invalid config.mk file because the LLDB version on the test bots is too old :/

@michaelwoerister
Copy link
Member Author

I've just pushed an update to this. configure should now handle the case more gracefully where --python-path option isn't yet available in LLDB. I also added the requested comment at the top of lldb_batchmode.py. Let's give it another try :)

@alexcrichton
Copy link
Member

Needs a rebase already! :(

@michaelwoerister
Copy link
Member Author

Done!

@sfackler
Copy link
Member

sfackler commented May 7, 2014

Still needs a rebase? Unless Github hasn't caught up yet.

@michaelwoerister
Copy link
Member Author

Done. Thanks for the heads up. I would have waited for some kind of more visible hint, that this can't be merged automatically...

bors added a commit that referenced this pull request May 7, 2014
…ichton

This pull request contains preparations for adding LLDB autotests:
+ the debuginfo tests are split into debuginfo-gdb and debuginfo-lldb
  + the `compiletest` tool is updated to support the debuginfo-lldb mode
  + tests.mk is modified to provide debuginfo-gdb and debuginfo-lldb make targets
  + GDB test cases are moved from `src/test/debug-info` to `src/test/debuginfo-gdb`
+ configure will now look for LLDB and set the appropriate CFG variables
+ the `lldb_batchmode.py` script is added to `src/etc`. It emulates GDB's batch mode

The LLDB autotests themselves are not part of this PR. Those will probable require some manual work on the test bots to make them work for the first time. Better to get these unproblematic preliminaries out of the way in a separate step.
@bors bors closed this May 7, 2014
@brson
Copy link
Contributor

brson commented May 7, 2014

\o/

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 29, 2021
…lacrum

compiletest: Remove some vestigial code

The `check_lines` header is no longer parsed as a header, but instead inside the debuginfo tests. I believe this was changed in rust-lang#13726.
arcnmx pushed a commit to arcnmx/rust that referenced this pull request Dec 17, 2022
feat: allow unwrap block in let initializers

Possible fix for rust-lang#13679

### Points to help in review:

- I just added a parent case for let statements and it seems everything else was in place already, so turned out to be a small fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants