[PATCH] D114415: [llvm] [Support] Add HTTP Server Support library.

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

I think this is mostly OK. (still, might be worth initializing/deinitializing in test fixtures, to isolate the tests a bit more - but probably not a big deal/not the sort of bugs we're likely to see (eg: where one test taints the libcurl state and another fails in weird ways because of that taint))



================
Comment at: llvm/unittests/Support/HTTPServer.cpp:82
+  EXPECT_EQ(Buffer.Code, Response.Code);
+  EXPECT_EQ(Buffer.Body->MemoryBuffer::getBuffer(), Response.Body);
+  Server.stop();
----------------
noajshu wrote:
> dblaikie wrote:
> > The `MemoryBuffer::` probably isn't needed here, I'd have thought?
> Ah, sadly it is required, unless you can see a better way around the following.
> The `Body` is a `WritableMemoryBuffer`, whose override of `getBuffer` returns an `MutableArrayRef<char>` (as opposed to the parent's virtual method `MemoryBuffer::getBuffer` which returns a `StringRef`). For some reason there is no `operator==` implemented between `MutableArrayRef<char>` and `std::string`. So we have to manually specify use of the parent method
Seems like addressing the missing comparison would be good as a separate patch.


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

https://reviews.llvm.org/D114415