labath 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
----------------
xiaobai wrote:
> 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?
Sounds like a plan.
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
