-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: add end-to-end tests using fake upstreams #67
Conversation
cc @dianwen - these are the end-to-end tests I mentioned. Cross-posting from Slack to make sure they don't get lost in the noise and you have a chance to take a look if you'd like. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥳
defer upstream.Close() | ||
|
||
upstreamConfigs := []config.UpstreamConfig{ | ||
{ID: "testNode", HTTPURL: upstream.URL}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious to what these test URLs looks like 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it listens to 127.0.0.1:0
by default
TIL: Port 0 is a wildcard port that tells the system to find a suitable port number.
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestServeHTTP_ForwardsToSoleHealthyUpstream(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These kind of remind me of a halfway point between unit tests and an integration tests!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, they don't bring up a full set of infrastructure (real ETH nodes, etc.) so they're not 100% realistic, but it's super convenient that they're all written in the same language (i.e. more control, better tooling) and they're also reasonably efficient to run, unlike a full-blown setup.
upstream := setUpHealthyUpstream(t, map[string]func(t *testing.T, w http.ResponseWriter){}) | ||
defer upstream.Close() | ||
|
||
upstreamConfigs := []config.UpstreamConfig{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to include one unhealthy node in this config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added!
Description
This PR adds basic support for running end-to-end tests that cover the whole routing. It uses in-process servers from the
net/http/httptest
package to bring up fake upstreams, and returns legitimately-looking responses to health check requests. Each test can then configure additional handlers for other RPC methods and check that they are routed as expected.Notable changes:
IsInitialized
methods toRouter
andHealthCheckManager
. I needed a way to confirm that the gateway is fully up and running - in particular, that at least one iteration of the health checks has already executed. Without this, we can't be sure if the behaviour we're testing actually respects the health checks./health
endpoint to reflect the above. I thought I was going to need that for the test, but I realized I can directly poll on therouter
instead. However, this still seemed like the correct thing to do - for example, if I'm doing a blue-green deploy, I want to make sure that my newly deployed gateway instance is fully up and running before I redirect traffic to it. The previous health check didn't support this.Type of change
How Has This Been Tested?
Added new tests and confirmed they pass (and also confirmed that they break accordingly when I break the underlying behaviour).