Improve API client tests with a mock HTTP server

[mkeeler]

Right now API client "unit" tests require a full agent to be running for testing. We should improve this by having a mock HTTP server which can respond to our API requests without actually running any agent code.

We will still need integration tests to run against the actual binary but hopefully we can do that in a way where we can point it at a random consul version and let it run which would improve our backwards compat testing.

This mock would also allow us to test some of the enterprise endpoints with stubs in OSS without actually pulling the full endpoint into oss (like for licensing).

[s-mang]

A concrete takeaway from this new mock server should be that it makes writing a test for this PR (e.g.) https://github.com/hashicorp/consul/pull/6311 much easier to do. (note to self)

[EvgeniaMartynova]

Hi Matt, Is any update on this?

[dnephin]

The way that I would generally test API clients like this would be, for each CLI command:

A small number (1-2) end-to-end tests for the CLI that test the happy path. Maybe a few more for really complex or core commands. These would run the agent binary, but could run the CLI command in-process. One of these should be a "kitchen sink" test that tries to set as many flags or config fields as possible. Running the agent binary allows for cross-version testing (as mentioned in the OP).

A larger number of unit tests that work against either a mock client, or a stubbed server. These tests would cover all the error cases, and any other client side logic. We don't necessarily need the same solution for every command. As long as the commands can be constructed in a way that allows us to point at a fake.

I imagine many tests would be fine with a hard-coded response for the individual test. The hard-coded response could be returned from https://golang.org/pkg/net/http/httptest/, or could be from a fake API client. Maybe a few complex cases might benefit from something like https://github.com/dnaeon/go-vcr, but in most cases I expect that would be unnecessary.

Since our CLI commands are split up into many small packages, we probably don't need to spend much time changing the existing tests. We can already run a subset of tests quite easily, and CI can split up the packages to run in parallel to keep the CI runtime fast. At most we might add a if testing.Short { t.Skip(...) } to the existing integration tests, so that we can skip them during local development.

The next time we need to write a test for the CLI, I think the following should work:

// handler returns a hard-coded response
handler := http.HandlerFunc(...)
srv := httptest.NewServer(handler)
t.Cleanup(srv.Close)

cmd.http.address = &srv.URL
err := cmd.Run(...)
...

As we write more of these we may find value in sharing some of the fake handlers, or we may find that they are easy enough to write that keeping the values local to the test makes them easier to work with.