End-to-end tests for service registration #858
Conversation
The existing TestService struct wasn't sufficient because it uses Name and not Service for the service name. There are also addiitonal attributes on a service that we may want to validate in the future.
mkam
requested review from
a team and
wilkermichael
and removed request for
a team
e2e/registration_test.go
Outdated
| func TestE2E_ServiceRegistration_RegistrationFailure(t *testing.T) { | ||
| // TestE2E_ServiceRegistration_RegistrationSkipped tests scenarios where registration does | ||
| // not happen, but CTS continues to run. | ||
| func TestE2E_ServiceRegistration_RegistrationSkipped(t *testing.T) { |
There was a problem hiding this comment.
The diff for this commit is hard to read, but I basically just put the old test in a for loop where the only difference between the test cases is the service_registration config. From a test perspective, all the setup and validation was the same whether the config disabled the registration or the config caused an error with registration.
testutils/consul.go
Outdated
| return checks | ||
| } | ||
|
|
||
| func WaitForCheckStatus(tb testing.TB, srv *testutil.TestServer, checkID, status string, wait time.Duration) (Check, error) { |
There was a problem hiding this comment.
Something I've noted is that we have a lot of WaitFor methods that all have the same structure. I'd like to refactor this in the future so that they all use a common method for polling, kind of in the same way we have a generic retry structure in the CTS code. I'll be making an issue for this.
There was a problem hiding this comment.
Opened #859 to address this.
| params.Add("filter", filter) | ||
| u = fmt.Sprintf("%s?%s", u, params.Encode()) | ||
| } | ||
| resp := RequestHTTP(tb, http.MethodGet, u, "") |
There was a problem hiding this comment.
You don't have to for this PR, but might be useful to wrap this in a retry
There was a problem hiding this comment.
Could also be useful to add ListServices to the Consul client, and use the API library instead of the raw get query: https://pkg.go.dev/github.com/hashicorp/consul/api#Agent.ServicesWithFilter
There was a problem hiding this comment.
Might also be useful to call this ConsulListServices since it's testutils.ListServices currently
e2e/registration_test.go
Outdated
| @@ -150,6 +151,51 @@ func TestE2E_ServiceRegistration_DeregisterWhenStopped(t *testing.T) { | |||
| assert.False(t, registered) | |||
| } | |||
|
|
|||
| // TestE2E_ServiceRegistration_RegistrationFailure tests that if CTS is unable | |||
| // to register itself as a service, it will not exit. | |||
| func TestE2E_ServiceRegistration_RegistrationFailure(t *testing.T) { | |||
There was a problem hiding this comment.
Maybe include the test case being tested in the name?
TestE2E_ServiceRegistration_ContinueOnRegistrationFailure or something like that
Also adds comments to the exported methods and structs.
mkam
force-pushed
the
self-registration-e2e
branch
from
8a4787a to
7d665c2
Compare
Frees resources by adding the small wait.
mkam
force-pushed
the
self-registration-e2e
branch
from
7d665c2 to
357fb07
Compare
mkam
force-pushed
the
self-registration-e2e
branch
from
357fb07 to
4d7b280
Compare
Adds e2e tests for service registration and various additional Consul testutil methods to verify the registered service and health check.
Sorry for the extensive PR, there were a lot of changes needed to support the various use cases. An overview of the non-test changes are:
Consul testutil additions:
Config changes:
idconfigurationModule additions:
And this wasn't really a part of the tests, but I wanted to make some improvements to the retry logging which I noted while testing the use case where registration fails with a non-retryable error.
this error is not retryablesince the userCloses #860