proxy: Make `outbound_updates_newer_services` test forward-compatible (#939)

This is in preparation for landing the Tokio upgrade.

The test `discovery::outbound_updates_newer_services` currently contains an
assertion that an HTTP/2 request to an HTTP/1 service will return a response
with status code 500. This is because the current version of Hyper on which
Conduit depends does not support protocol upgrades.

However, commit hyperium/hyper@bc6af88a32, which
adds support for this kind of protocol upgrade, was recently merged to Hyper's
master branch. Therefore, this assertion will no longer be correct once we 
depend on the upcoming Hyper release. When we migrate to the new Tokio, it will
be necessary to upgrade our Hyper dependency as well, and this test will fail.

I've modified the test to no longer make assertions about the response's status
code, so that it's compatible with both the current and future Hyper versions.
If the response is not `Ok`, the test will still fail, since 
`tests::support::Client::request()` `expect`s that the response is successful,
but the status code is ignored. I've added a comment in the test explaining 
this.

Eventually, when the master version of Conduit depends on the latest Hyper, we
may want to change this test to assert that the status code is 200 instead. We
may also want to add more tests for Hyper's protocol upgrade functionality, but
that seems out of scope for this PR.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This commit is contained in:
Eliza Weisman 2018-05-11 14:36:03 -07:00 committed by GitHub
parent 030bd404fa
commit 281281f5bc
1 changed files with 6 additions and 12 deletions

View File

@ -185,8 +185,6 @@ mod http1 {
fn outbound_updates_newer_services() {
let _ = env_logger::try_init();
//TODO: when the support server can listen on both http1 and http2
//at the same time, do that here
let srv = server::http1().route("/h1", "hello h1").run();
let ctrl = controller::new()
@ -201,16 +199,12 @@ fn outbound_updates_newer_services() {
// from the controller
let client1 = client::http2(proxy.outbound, "disco.test.svc.cluster.local");
// This HTTP2 client tries to talk to our HTTP1 server, and the server
// will return an error (see above TODO).
//
// The reason to do this request at all is because the proxy will create
// an H2 service mapping when it sees an H2 request, and we want to test
// that when it sees H1 and tries to create a new mapping, the existing
// known Discovery information is shared with it.
let res = client1.request(&mut client1.request_builder("/h1"));
assert_eq!(res.status(), 500);
// Depending on the version of `hyper` we're using, protocol upgrades may or
// may not be supported yet, so this may have a response status of either 200
// or 500. Ignore the status code for now, and just expect that making the
// request doesn't *error* (which `client.request` does for us).
let _res = client1.request(&mut client1.request_builder("/h1"));
// assert_eq!(res.status(), 200);
// a new HTTP1 service needs to be build now, while the HTTP2
// service already exists, so make sure previously sent addrs