Skip to content

Comments

sd/eureka: remove data races in unit tests#547

Merged
peterbourgon merged 2 commits intogo-kit:masterfrom
seh:remove-eureka-test-race
Jun 8, 2017
Merged

sd/eureka: remove data races in unit tests#547
peterbourgon merged 2 commits intogo-kit:masterfrom
seh:remove-eureka-test-race

Conversation

@seh
Copy link

@seh seh commented Jun 7, 2017

Addressing #545, revise the fake fargo components used in the Eureka-related tests to behave more like a real Eureka server, and reduce the tests' vulnerability to delays in asynchronously-arriving responses from the server by polling, with a limit in place to make the tests fail after polling for one second.

This proposal changes eureka.Instancer's cache priming implementation, though the behavior observable by callers shouldn't change.

Steven E. Harris added 2 commits June 7, 2017 10:05
Rather than first fetching a Eureka application's instances
synchronously and then scheduling subsequent background updates,
instead schedule the updates, wait on the first update to arrive, an
then proceed.

Revise the tests accordingly to no longer rely on a set of instances
known to the (fake) server as distinct from any applications. Since
applications are an intensional view of instances, like the real
Eureka server we derive the presence of applications from the
application names mentioned by instances. That is, without at least
one instance declaring that it's a member of a given application, no
such application exists.
The fargo library already provides the HTTPResponseStatusCode function
to extract an HTTP status code from a returned error, but here we want
to be able to inject similar errors for tests, so look for both a real
fargo error indicating that a heartbeat attempt failed to find its
target instance and a synthetic error returned by a fake connection
during a test.
@peterbourgon
Copy link
Member

Closes #545.

Nice!

@peterbourgon peterbourgon merged commit 714eef7 into go-kit:master Jun 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants