labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.
================
Comment at:
lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py:5
+
+class StepIntoNamespace(TestBase):
+ mydir = TestBase.compute_mydir(__file__)
----------------
Am I correct in assuming that the "namespace" part here comes from the fact
that global functions (e.g. `_Z3foov` aka `foo()`) would be found according to
their base name (`foo`) or something?
If yes, that's fine, but it would still be nice to mention (say in a comment or
something) that the shared library is an important aspect of the test
("TestStepIntoNamespace" does not convey that notion).
If this can also be reproduced with regular functions, then it'd be better to
rename the test into something else (which includes the word "trampoline").
================
Comment at:
lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py:8
+
+ def test(self):
+ self.build()
----------------
mdaniels wrote:
> clayborg wrote:
> > Do we want to limit this to linux? I am not sure this will pass the windows
> > buildbots if we don't restrict it
> Looking at other tests that load a shared library, it seems I should have at
> least
>
> ```
> @skipIfRemote
> @skipIfWindows
> ```
> But I can also just restrict it to the platforms I am able to test on
> locally, linux and darwin, if there is still concern that other platforms
> might fail.
Neither of these is strictly necessary. Remote shared library tests need
additional care as one has to copy the shared library to the remote system, but
afaik, you're using the correct incantation which should do that automatically.
Windows shared library support is not at the level of other platforms, but a
simple setup like this should work. Or rather: if it fails, it will probably be
due to the trampoline aspect, and not the shared library loading. So I think
you don't need to add any decorators right now. We can add xfail-windows if it
turns out to be failing.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127999/new/
https://reviews.llvm.org/D127999
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
