xiaobai marked an inline comment as done.
xiaobai added inline comments.
================
Comment at: packages/Python/lldbsuite/test/functionalities/paths/TestPaths.py:20
+ # GetClangResourceDir doesn't work on windows yet
+ @expectedFailureAll(oslist=["windows"])
@no_debug_info_test
----------------
labath wrote:
> xiaobai wrote:
> > teemperor wrote:
> > > Doesn't that mean that we no longer test any of the paths on windows?
> > > E.g. if someone breaks `lldb.ePathTypeLLDBShlibDir` we will not see this
> > > test failing on Windows. I would prefer of extracting the ClangDir test
> > > into it's own method that we mark as failing.
> > Technically it is being tested but it is expected to fail. That being said,
> > I don't mind extracting the ClangDir test into its own method while I work
> > on implementing it, but I don't think it needs its own test. Maybe after I
> > implement `GetClangResourceDir` on windows I can merge the tests? :P
> That sounds fine, though I have to say that these tests don't really test
> much. There is a test for the MacOS functionality here in
> unittests/Expression/ClangParserTest.cpp, which is much more detailed. It
> would be great if you could add a windows version there too.
I originally added this test there, but testing things there as-is is presents
a disadvantage: The unittests don't link against liblldb but the individual
lldb libraries, so the methods that figure out where the Clang Resource Dir
(and pretty much any other directory for that matter) will report something
like this:
`/path/to/llvm/build/tools/lldb/unittests/lib/clang/9.0.0`. So, the best this
test could do is set that the filespec had a directory and not a filename set,
which is what this test already does.
That being said, the way MacOS is being tested is much nicer. I could do a
small refactor so the non-MacOS posix implementation is closer to the MacOS
implementation and add a Linux test (and eventually a windows test) to
`unittests/Expression/ClangParserTest.cpp` as well. What do you think?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58748/new/
https://reviews.llvm.org/D58748
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
