Localize mock server dependency to HttpClient test

The mock server is not intended to serve requests/responses for all
tests. We only intend to use it to ensure that we handle URLs, query
parameters, headers, and general HTTP matters correctly.

Everything "outward" from here should use mocks, generally, out to the
system tests, which will use a real API. The HttpClient response bodies
can be mocked inline in the ApiClient tests for locality and ease. The
mock really server only needs to run for http_client_test.cpp.

apache/client/test/lauth/api_client_test.cpp

@@ -1,5 +1,6 @@
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
+
 #include "mocks.hpp"
 
 using ::testing::_;
@@ -11,7 +12,7 @@ using ::testing::Return;
 
 using namespace mlibrary::lauth;
 
-TEST(ApiClient, allowed_by_mock_http_client) {
+TEST(ApiClient, RequestByAuthorizedUserIsAllowed) {
   auto client = std::make_unique<MockHttpClient>();
   EXPECT_CALL(*client, get("/users/authorized/is_allowed")).WillOnce(Return("yes"));
   ApiClient api_client(std::move(client));
@@ -27,7 +28,7 @@ TEST(ApiClient, allowed_by_mock_http_client) {
   EXPECT_THAT(allowed, true);
 }
 
-TEST(ApiClient, denied_by_mock_http_client) {
+TEST(ApiClient, RequestByUnauthorizedUserIsDenied) {
   auto client = std::make_unique<MockHttpClient>();
   EXPECT_CALL(*client, get("/users/unauthorized/is_allowed")).WillOnce(Return("no"));
   ApiClient api_client(std::move(client));
@@ -43,29 +44,24 @@ TEST(ApiClient, denied_by_mock_http_client) {
   EXPECT_THAT(allowed, false);
 }
 
-TEST(ApiClient, a_request_with_no_user_is_denied) {
-  ApiClient client("http://localhost:9000");
+TEST(ApiClient, RequestByUnknownUserIsDenied) {
+  GTEST_SKIP() << "This is passing for the wrong reason and GMock is giving "
+               << "a warning because we have not set any expectations. "
+               << "There is an uninteresting/unexpected call to the HttpClient "
+               << "to get '/users//is_allowed', which should be rejected "
+               << "before the HTTP request or expressed differently (possibly "
+               << "with query params).";
+  auto http_client = std::make_unique<MockHttpClient>();
+  ApiClient client(std::move(http_client));
   Request request;
 
   bool result = client.isAllowed(request);
   EXPECT_THAT(result, false);
 }
 
-
-TEST(ApiClient, a_request_with_authorized_user_is_allowed) {
-  ApiClient client("http://localhost:9000");
-  Request request;
-
-  request.user = "authorized";
-  bool result = client.isAllowed(request);
-  EXPECT_THAT(result, true);
-}
-
-TEST(ApiClient, a_request_with_unauthorized_user_is_denied) {
-  ApiClient client("http://localhost:9000");
-  Request request;
-
-  request.user = "unauthorized";
-  bool result = client.isAllowed(request);
-  EXPECT_THAT(result, false);
-}
+TEST(ApiClient, UsesTheSuppliedApiUrl) {
+  GTEST_SKIP() << "Skipping test that ApiClient makes an HttpClient for the "
+               << "correct URL... pushing everything back to config and likely "
+               << "a factory/builder rather than concrete class dependency.";
+  ApiClient client("http://api.invalid");
+}

apache/client/test/lauth/http_client_test.cpp

@@ -1,45 +1,52 @@
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
 
-using testing::_;
-using testing::Eq;
-using testing::IsTrue;
-
 #include <httplib.h>
 
+#include "mocks.hpp"
+
 #include "lauth/http_client.hpp"
 #include "lauth/request.hpp"
 
+#include <string>
 #include <cstdlib>
 
-using namespace mlibrary::lauth;
+using testing::_;
+using testing::Eq;
+using testing::IsTrue;
 
-const std::string LOCAL_API_URL = "http://localhost:9000";
+using namespace mlibrary::lauth;
 
-const std::string TEST_API() {
-  if (const char *env_url = std::getenv("LAUTH_TEST_API_URL"))
-    return std::string(env_url);
-  else
-    return LOCAL_API_URL;
+const std::string LOCAL_MOCK_API_URL = "http://localhost:9000";
+
+const static std::string& MOCK_API_URL() {
+    static std::string url;
+    if (url.empty()) {
+        if (const char *env_url = std::getenv("LAUTH_TEST_API_URL"))
+            url = std::string(env_url);
+        else
+            url = LOCAL_MOCK_API_URL;
+    }
+    return url;
 }
 
-TEST(HttpClient, tests_require_mock_api) {
-  HttpClient client(TEST_API());
+TEST(HttpClient, uses_mock_server) {
+  HttpClient client(MOCK_API_URL());
 
   auto response = client.get("/");
-  ASSERT_THAT(response.has_value(), IsTrue()) << "Test server does not appear responsive at "
-    << TEST_API() << ". Is it running? Have you set LAUTH_TEST_API_URL correctly?";
+  ASSERT_THAT(response.has_value(), IsTrue()) << "Mock server does not appear responsive at "
+    << MOCK_API_URL() << ". Is it running? Have you set LAUTH_TEST_API_URL correctly?";
 }
 
 TEST(HttpClient, get_request_returns_body) {
-  HttpClient client(TEST_API());
+  HttpClient client(MOCK_API_URL());
 
   auto response = client.get("/");
   EXPECT_THAT(response, Eq("Root"));
 }
 
 TEST(HttpClient, get_request_with_path_returns_body) {
-  HttpClient client(TEST_API());
+  HttpClient client(MOCK_API_URL());
 
   auto response = client.get("/ping");
   EXPECT_THAT(response, "pong");

apache/client/test/lauth/mocks.hpp

@@ -8,16 +8,16 @@
 
 using namespace mlibrary::lauth;
 
-class MockApiClient : public ApiClient {
+class MockHttpClient : public HttpClient {
     public:
-    MockApiClient() : ApiClient("http://localhost:9000") {};
-    MOCK_METHOD(bool, isAllowed, (Request), (override));
+    MockHttpClient() : HttpClient("http://api.invalid") {};
+    MOCK_METHOD(std::optional<std::string>, get, (const std::string&), (override));
 };
 
-class MockHttpClient : public HttpClient {
+class MockApiClient : public ApiClient {
     public:
-    MockHttpClient() : HttpClient("http://localhost:9000") {};
-    MOCK_METHOD(std::optional<std::string>, get, (const std::string&), (override));
+    MockApiClient() : ApiClient(std::make_unique<MockHttpClient>()) {};
+    MOCK_METHOD(bool, isAllowed, (Request), (override));
 };
 
 #endif