Repository: roapi/roapi (language: python)

Issue:
Add HTTP2 integration test in CI
to make sure we don't have regression on dropping support for http2 going forward. The test can just be a simple http2 curl call to roapi.


Discussion / hints:
Hey @houqp happy to take this on next if that's ok? Please feel free to assign to me. 
sure, thanks!
So I've played around with this a bit and early indications are that testing `localhost:8000` for HTTP/2 is not likely to yield results unless we figure out how to run up a server with TLS. 

For example:

```
~ % curl -v --http2 localhost:8080/api/schema
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /api/schema HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.80.0
> Accept: */*
> Connection: Upgrade, HTTP2-Settings
> Upgrade: h2c
> HTTP2-Settings: AAMAAABkAAQCAAAAAAIAAAAA
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< content-type: application/json
< content-length: 6579
< date: Fri, 03 Dec 2021 06:22:48 GMT
<
{"uk_cities":{"fields":[<snip>
```
Update: I've got a possible solution that I would like to sketch out. Essentially I would like to do the following in an integration test:

```
~ % curl -sI --http2-prior-knowledge localhost:8080/api/schema -o/dev/null -w '%{http_version}\n'
2
```

So I will check that the curl command there will return `2` (it returns `1.1` if the server does not support HTTP/2).

Notes:

When you use `http2-prior-knowledge` you are essentially saying that I expect my server to respond with HTTP/2 and I do not want to degrade to HTTP/1.1. See: 

https://everything.curl.dev/http/http2
https://curl.se/docs/http2.html#:~:text=curl%20offers%20the%20%2D%2Dhttp2%2Dprior%2Dknowledge%20command%20line%20option,by%20default%20for%20HTTPS%20connections.
@novemberkilo There are already some integration tests that use Reqwest. There is an option to set the HTTP version on the request: https://docs.rs/reqwest/latest/reqwest/struct.RequestBuilder.html#method.version

Maybe that will do as well?
that `http2-prior-knowledge` option is very cool, thanks @novemberkilo for discovering that :D . I am cool with us going either way (curl v.s. reqwest) as long as we are testing http2 end to end.

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 78e241401..40f145dcb 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -101,6 +101,59 @@ version = "0.9.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "eab1c04a571841102f5345a8fc0f6bb3d31c315dec879b5c6e42e40ce7ffa34e"
 
+[[package]]
+name = "async-channel"
+version = "1.6.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2114d64672151c0c5eaa5e131ec84a74f06e1e559830dabba01ca30605d66319"
+dependencies = [
+ "concurrent-queue",
+ "event-listener",
+ "futures-core",
+]
+
+[[package]]
+name = "async-io"
+version = "1.6.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a811e6a479f2439f0c04038796b5cfb3d2ad56c230e0f2d3f7b04d68cfee607b"
+dependencies = [
+ "concurrent-queue",
+ "futures-lite",
+ "libc",
+ "log",
+ "once_cell",
+ "parking",
+ "polling",
+ "slab",
+ "socket2",
+ "waker-fn",
+ "winapi",
+]
+
+[[package]]
+name = "async-process"
+version = "1.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "83137067e3a2a6a06d67168e49e68a0957d215410473a740cea95a2425c0b7c6"
+dependencies = [
+ "async-io",
+ "blocking",
+ "cfg-if 1.0.0",
+ "event-listener",
+ "futures-lite",
+ "libc",
+ "once_cell",
+ "signal-hook",
+ "winapi",
+]
+
+[[package]]
+name = "async-task"
+version = "4.0.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e91831deabf0d6d7ec49552e489aed63b7456a7a3c46cff62adad428110b0af0"
+
 [[package]]
 name = "async-trait"
 version = "0.1.51"
@@ -112,6 +165,12 @@ dependencies = [
  "syn",
 ]
 
+[[package]]
+name = "atomic-waker"
+version = "1.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "065374052e7df7ee4047b1160cca5e1467a12351a40b3da123c870ba0b8eda2a"
+
 [[package]]
 name = "atty"
 version = "0.2.14"
@@ -210,6 +269,20 @@ dependencies = [
  "generic-array",
 ]
 
+[[package]]
+name = "blocking"
+version = "1.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "046e47d4b2d391b1f6f8b407b1deb8dee56c1852ccd868becf2710f601b5f427"
+dependencies = [
+ "async-channel",
+ "async-task",
+ "atomic-waker",
+ "fastrand",
+ "futures-lite",
+ "once_cell",
+]
+
 [[package]]
 name = "brotli"
 version = "3.3.2"
@@ -261,6 +334,12 @@ version = "1.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "c4872d67bab6358e59559027aa3b9157c53d9358c51423c17554809a8858e0f8"
 
+[[package]]
+name = "cache-padded"
+version = "1.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "631ae5198c9be5e753e5cc215e1bd73c2b466a3565173db433f52bb9d3e66dba"
+
 [[package]]
 name = "cc"
 version = "1.0.72"
@@ -421,6 +500,15 @@ dependencies = [
  "unicode-width",
 ]
 
+[[package]]
+name = "concurrent-queue"
+version = "1.2.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "30ed07550be01594c6026cff2a1d7fe9c8f683caa798e12b68694ac9e88286a3"
+dependencies = [
+ "cache-padded",
+]
+
 [[package]]
 name = "const_fn"
 version = "0.4.8"
@@ -740,6 +828,21 @@ dependencies = [
  "str-buf",
 ]
 
+[[package]]
+name = "event-listener"
+version = "2.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f7531096570974c3a9dcf9e4b8e1cede1ec26cf5046219fb3b9d897503b9be59"
+
+[[package]]
+name = "fastrand"
+version = "1.5.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b394ed3d285a429378d3b384b9eb1285267e7df4b166df24b7a6939a04dc392e"
+dependencies = [
+ "instant",
+]
+
 [[package]]
 name = "fd-lock"
 version = "3.0.0"
@@ -859,6 +962,21 @@ version = "0.3.17"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "522de2a0fe3e380f1bc577ba0474108faf3f6b18321dbf60b3b9c39a75073377"
 
+[[package]]
+name = "futures-lite"
+version = "1.12.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7694489acd39452c77daa48516b894c153f192c3578d5a839b62c58099fcbf48"
+dependencies = [
+ "fastrand",
+ "futures-core",
+ "futures-io",
+ "memchr",
+ "parking",
+ "pin-project-lite",
+ "waker-fn",
+]
+
 [[package]]
 name = "futures-macro"
 version = "0.3.17"
@@ -1132,6 +1250,15 @@ dependencies = [
  "hashbrown",
 ]
 
+[[package]]
+name = "instant"
+version = "0.1.12"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7a5bbe824c507c5da5956355e86a746d82e0e1464f65d862cc5e71da70e94b2c"
+dependencies = [
+ "cfg-if 1.0.0",
+]
+
 [[package]]
 name = "integer-encoding"
 version = "1.1.7"
@@ -1629,6 +1756,12 @@ dependencies = [
  "libm",
 ]
 
+[[package]]
+name = "parking"
+version = "2.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "427c3892f9e783d91cc128285287e70a59e206ca452770ece88a76f7a3eddd72"
+
 [[package]]
 name = "parquet"
 version = "6.2.0"
@@ -1709,6 +1842,19 @@ version = "0.3.22"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "12295df4f294471248581bc09bef3c38a5e46f1e36d6a37353621a0c6c357e1f"
 
+[[package]]
+name = "polling"
+version = "2.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "685404d509889fade3e86fe3a5803bca2ec09b0c0778d5ada6ec8bf7a8de5259"
+dependencies = [
+ "cfg-if 1.0.0",
+ "libc",
+ "log",
+ "wepoll-ffi",
+ "winapi",
+]
+
 [[package]]
 name = "ppv-lite86"
 version = "0.2.15"
@@ -1978,6 +2124,7 @@ name = "roapi-http"
 version = "0.5.1"
 dependencies = [
  "anyhow",
+ "async-process",
  "axum",
  "clap",
  "columnq",
@@ -2455,6 +2602,16 @@ version = "1.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "43b2853a4d09f215c24cc5489c992ce46052d359b5109343cbafbf26bc62f8a3"
 
+[[package]]
+name = "signal-hook"
+version = "0.3.10"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9c98891d737e271a2954825ef19e46bd16bdb98e2746f2eec4f7a4ef7946efd1"
+dependencies = [
+ "libc",
+ "signal-hook-registry",
+]
+
 [[package]]
 name = "signal-hook-registry"
 version = "1.4.0"
@@ -3077,6 +3234,12 @@ version = "1.0.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d"
 
+[[package]]
+name = "waker-fn"
+version = "1.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9d5b2c62b4012a3e1eca5a7e077d13b3bf498c4073e33ccd58626607748ceeca"
+
 [[package]]
 name = "want"
 version = "0.3.0"
@@ -3188,6 +3351,15 @@ dependencies = [
  "webpki",
 ]
 
+[[package]]
+name = "wepoll-ffi"
+version = "0.1.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d743fdedc5c64377b5fc2bc036b01c7fd642205a0d96356034ae3404d49eb7fb"
+dependencies = [
+ "cc",
+]
+
 [[package]]
 name = "winapi"
 version = "0.3.9"
diff --git a/roapi-http/Cargo.toml b/roapi-http/Cargo.toml
index a4469f647..f406150bf 100644
--- a/roapi-http/Cargo.toml
+++ b/roapi-http/Cargo.toml
@@ -48,6 +48,7 @@ snmalloc = ["snmalloc-rs"]
 
 [dev-dependencies]
 reqwest = { version = "0.11", default-features = false, features = ["json", "rustls-tls"]}
+async-process = "1.3.0"
 
 # TODO: uncomment this when we exclude roapi-http from root workspace
 # [profile.release]


Test patch:
diff --git a/roapi-http/tests/api_test.rs b/roapi-http/tests/api_test.rs
index 2cf5ab9c4..be8584875 100644
--- a/roapi-http/tests/api_test.rs
+++ b/roapi-http/tests/api_test.rs
@@ -3,6 +3,7 @@ mod helpers;
 use std::collections::HashMap;
 
 use anyhow::Result;
+use async_process::Command;
 use columnq::arrow::datatypes::Schema;
 use tokio;
 
@@ -231,3 +232,34 @@ async fn test_graphql_post_selection() -> Result<()> {
     );
     Ok(())
 }
+
+#[tokio::test]
+async fn test_http2() -> Result<()> {
+    let table = helpers::get_uk_cities_table();
+    let (app, address) = helpers::test_api_app(vec![table]).await;
+    tokio::spawn(app.run_until_stopped());
+
+    // ~ % curl -sI --http2-prior-knowledge localhost:8080/api/schema -o/dev/null -w '%{http_version}\n'
+    // 2
+
+    let http_version = Command::new("curl")
+        .arg("-s")
+        .arg("-I")
+        .arg("--http2-prior-knowledge")
+        .arg(format!("{}/api/schema", address))
+        .arg("-o")
+        .arg("/dev/null")
+        .arg("-w")
+        .arg("'%{http_version}\n'")
+        .output()
+        .await?
+        .stdout;
+
+    let two = vec!['\'', '2', '\n', '\'']
+        .iter()
+        .map(|c| *c as u8)
+        .collect::<Vec<_>>();
+
+    assert_eq!(http_version, two);
+    Ok(())
+}