Skip to content

Commit 3eaacab

Browse files
committed
test(rust): trim duplicate coverage in the proxy test suite
The 2xx-that-is-not-200 tripwire looped over 201 and 204, asserting the identical fact twice: both exercise the same "not literally 200" check in hyper_util's Tunnel. Kept 201 alone. Also shares the request-target parsing duplicated across two tests.
1 parent 3c7fd7d commit 3eaacab

1 file changed

Lines changed: 25 additions & 30 deletions

File tree

  • packages/rust/armonik-transport/tests

packages/rust/armonik-transport/tests/proxy.rs

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,7 @@ async fn serve_tunnel(
7474
stats: Arc<ProxyStats>,
7575
) -> std::io::Result<()> {
7676
let head = read_head(&mut client).await?;
77-
78-
let target = head
79-
.lines()
80-
.next()
81-
.and_then(|line| line.split_whitespace().nth(1))
82-
.unwrap_or_default()
83-
.to_owned();
77+
let target = request_target(&head);
8478

8579
if let ProxyAuth::Required(expected) = auth {
8680
let presented = head.lines().find_map(|line| {
@@ -109,6 +103,15 @@ async fn serve_tunnel(
109103
.map(|_| ())
110104
}
111105

106+
/// The request-target of a `CONNECT` request's start line, e.g. `proxy.corp:3128`.
107+
fn request_target(head: &str) -> String {
108+
head.lines()
109+
.next()
110+
.and_then(|line| line.split_whitespace().nth(1))
111+
.unwrap_or_default()
112+
.to_owned()
113+
}
114+
112115
/// Read up to the blank line that ends an HTTP head.
113116
async fn read_head(stream: &mut TcpStream) -> std::io::Result<String> {
114117
let mut head = Vec::new();
@@ -427,29 +430,27 @@ async fn a_dedicated_credential_in_system_mode_keeps_the_other_half_the_url_carr
427430
#[tokio::test]
428431
async fn known_issue_a_success_other_than_200_does_not_open_the_tunnel() {
429432
// RFC 9110: any 2xx switches the connection to tunnel mode. `hyper_util`'s `Tunnel`, which this
430-
// crate delegates the handshake to, checks for exactly `200`, so a proxy answering 201 or 204 is a
431-
// tunnel that should open and does not. See the crate README's "Known issues".
433+
// crate delegates the handshake to, checks for exactly `200`, so a proxy answering 201 is a tunnel
434+
// that should open and does not. See the crate README's "Known issues".
432435
//
433436
// A tripwire, not a preference: the day `hyper_util` accepts any 2xx, this starts failing, which
434437
// is the signal to loosen it back to asserting success and to update the README.
435438
//
436439
// Not asserted on `ProxyStats::tunnels`: the fake proxy counts a tunnel as soon as it has written
437440
// its own response, before learning whether the client accepted it, so that counter answers a
438441
// different question from the one this test asks.
439-
for success in [201u16, 204] {
440-
let server = spawn_server().await;
441-
let (proxy, _stats) = spawn_proxy_answering(ProxyAuth::None, success).await;
442-
443-
let error = call_through(through_proxy(&server, proxy, None))
444-
.await
445-
.expect_err(&format!("{success} unexpectedly opened the tunnel"));
446-
447-
assert!(
448-
error_chain(error.as_ref()).contains("did not open the tunnel"),
449-
"unexpected error for {success}: {}",
450-
error_chain(error.as_ref())
451-
);
452-
}
442+
let server = spawn_server().await;
443+
let (proxy, _stats) = spawn_proxy_answering(ProxyAuth::None, 201).await;
444+
445+
let error = call_through(through_proxy(&server, proxy, None))
446+
.await
447+
.expect_err("201 unexpectedly opened the tunnel");
448+
449+
assert!(
450+
error_chain(error.as_ref()).contains("did not open the tunnel"),
451+
"unexpected error: {}",
452+
error_chain(error.as_ref())
453+
);
453454
}
454455

455456
#[tokio::test]
@@ -510,13 +511,7 @@ async fn known_issue_a_portless_http_target_is_dialled_on_443_not_80() {
510511
let Ok(head) = read_head(&mut client).await else {
511512
return;
512513
};
513-
let target = head
514-
.lines()
515-
.next()
516-
.and_then(|line| line.split_whitespace().nth(1))
517-
.unwrap_or_default()
518-
.to_owned();
519-
*captured.lock().expect("lock") = Some(target);
514+
*captured.lock().expect("lock") = Some(request_target(&head));
520515
// No response: the client only needs to have sent the request to be observed here, and this
521516
// address would never answer regardless.
522517
});

0 commit comments

Comments
 (0)