This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Add test for unavailable registers
ClosedPublic

Authored by DavidSpickett on Mar 13 2023, 7:41 AM.

Details

Summary

Prior to this the only check was that we did not print
this message when reading registers that should exist.

I thought there was an indentation bug here so I wrote a test
for it. There is not, but we could do with the coverage anyway.

Diff Detail

Event Timeline

DavidSpickett created this revision.Mar 13 2023, 7:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 13 2023, 7:41 AM
DavidSpickett requested review of this revision.Mar 13 2023, 7:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 13 2023, 7:41 AM
DavidSpickett added inline comments.Mar 13 2023, 7:43 AM
lldb/test/API/commands/register/register/TestRegistersUnavailable.py
58

The only difference here if you have the x86 backend vs. not, is that there is another set after these two. I've checked the test in both configurations and it passes.

Seems like most xml-based tests like this are in lldb/test/API/functionalities/gdb_remote_client, is there a reason this is not in that package too?

It's not in that dir because it's testing how the register command handles missing registers, rather than how the gdb client does. It assumes that the gdb client is correctly doing its job.

And it seemed like the place one would look if you wondered whether there was an existing test.

The non XML way to do this is just reading bogus register names (and is already tested). However that's all or nothing so it doesn't prove that when there is a mix of available and unavailable registers the command works correctly.

DavidSpickett planned changes to this revision.Mar 21 2023, 10:00 AM

On second thought I think you can do this without XML, I'll figure that out.

Remove the need for regex, with the tradeoff that you need
the X86 backend which is pretty common to have.

rupprecht accepted this revision.Mar 21 2023, 12:17 PM
rupprecht added inline comments.
lldb/test/API/commands/register/register/TestRegistersUnavailable.py
44–54

optional: IIUC, patterns takes a list of patterns which are checked in order, so you could remove extraneous regexes like this one and pass a list instead.

This revision is now accepted and ready to land.Mar 21 2023, 12:17 PM
DavidSpickett marked an inline comment as done.Mar 22 2023, 2:41 AM
DavidSpickett added inline comments.
lldb/test/API/commands/register/register/TestRegistersUnavailable.py
44–54

That is how substrs work but regex is applied to the full output each time: https://github.com/llvm/llvm-project/blob/56d94a90dbbf1845ec71cd749691c74c1dd8a3ef/lldb/packages/Python/lldbsuite/test/lldbtest.py#L2382

So I will keep this as it is.

DavidSpickett marked an inline comment as done.Mar 22 2023, 2:41 AM
This revision was automatically updated to reflect the committed changes.