[PATCH] D112758: [llvm] [Debuginfo] Debuginfod client library.

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/unittests/Debuginfod/DebuginfodTests.cpp:13
+
+TEST(DebuginfodTests, noDebuginfodUrlsFetchInfoTest) {
+  EXPECT_THAT_EXPECTED(fetchDebuginfo("./", {}, "fakeBuildId",
----------------
noajshu wrote:
> noajshu wrote:
> > labath wrote:
> > > I suppose you could test here that the asset can be retrieved from the cache without hitting the server, but I don't think that's really necessary (if you have such a test in the next patch) -- a lot of library code in llvm is only tested through the relevant tool.
> > Currently there is no such test in the next patch. There are only lit tests where the cache is emptied each time and we fetch assets from a server. Is it OK to write to the filesystem in a unit test? If so I would like to add one here per your suggestion. If not, I can add a lit test to make sure local caching works properly.
> Thanks for the suggestion! I've added this test below.
> How about we divide up the testing responsibility like this:
> D112758 -- (this diff) -- Unit test a local cache hit and miss (done)
> D112759 -- (llvm-debuginfod-find) -- end-to-end test of local cache and server using Python simple http server
> D113717 -- (symbolizer integration) -- end-to-end test of local cache only (just to hit the integration point)
> 
> The goal would be to get full coverage (local cache, network, client integration point) without too much overlap across the tests.
Sounds good to me (I guess it's infeasible to test the network case in this patch (D112758) because it's a bunch more code to have a C++ server compared to the python http server that can be used once it's an actual command line tool, lit tested, etc as with llvm-debuginfod-find)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112758/new/

https://reviews.llvm.org/D112758