labath added inline comments.
================
Comment at:
lldb/test/Shell/SymbolFile/OnDemand/shared-lib-globals-hydration.test:4
+
+# REQUIRES: system-linux
+
----------------
yinghuitan wrote:
> labath wrote:
> > yinghuitan wrote:
> > > labath wrote:
> > > > Is this here because there is no portable way to create shared
> > > > libraries in shell tests? Would it be possible to run this test on
> > > > other systems as well if this were written as an API test (where we
> > > > have a shared lib portability layer)?
> > > Yeah, that's one reason. Another reason is the name of the shared library
> > > (used for checking global variables/debug info) varies across platform.
> > > For example, it can be libfoo.dylib (Mac) vs libfoo.so (linux) vs
> > > *foo*.dll (windows). I find it cumbersome to support multiple platforms.
> > I'm not saying you have to go out of your way to support other platforms
> > and their idiosyncrasies, but the differences in library names hardly seem
> > like an insurmountable obstacle. We already have abstractions in the API
> > tests (`self.platformContext.{shlib_prefix,shlib_extension}`) which exist
> > precisely for this reason. There's no function to automatically construct
> > the library name (you have to do the concatenation yourself), but I
> > certainly wouldn't be opposed to adding one.
> >
> > If it helps, think of it as courtesy towards other developers who may need
> > to debug this feature when they break these tests -- that's much easier to
> > achieve if you can reproduce the failure locally vs. just getting a failure
> > email from a random bot (after you've already submitted your patch).
> > self.platformContext.{shlib_prefix,shlib_extension}
>
> Ah, did not know their existence, thanks! Will add API tests for them. Do you
> prefer to keep these linux-only shell tests or completely deprecate them?
>
>
If they're going to be testing the exact same thing, then I don't see a reason
for keeping these around.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121631/new/
https://reviews.llvm.org/D121631
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
