[PATCH] D112751: [llvm] [Support] Add HTTP Client Support library.

[Noah Shutty via Phabricator via llvm-commits]

noajshu added a comment.

In D112751#3111697 <https://reviews.llvm.org/D112751#3111697>, @labath wrote:

> I think this is as good as it can get. What you do after that -- I don't know.
>
> It wasn't clear to me whether you forgot about the `mockHTTPClientBrokenConnectionTest` part, or chose not to do that, but I added an additional comment to describe how I would do this.



================
Comment at: llvm/unittests/Support/HTTPClient.cpp:140-141
+  using HTTPClient::perform;
+  Error perform(const HTTPRequest Request,
+                HTTPResponseHandler &Handler) override {
+    if (Error Err = Handler.handleStatusCode(200))
----------------
labath wrote:
> `MOCK_METHOD2(perform, Error(HTTPRequest, HTTPResponseHandler))`, and then just set expectations in the test. The entire `MockHTTPResponseHandler` business can go away.
Thanks! It seems the only library code we hit with these tests is the small convenience methods `perform(Request)` and `get(Url)`. Is it worth making the perform method virtual to be able to create the mock class?


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

https://reviews.llvm.org/D112751