Repository: bearcove/loona (language: rust)

Issue:
Bring hring-hpack into the year 2021
  * [x] Make sure all tests are run in CI (there's an optional feature using rustc-serialize, what's it about?
  * [x] Resolve existing warnings (unused functions, etc.)
  * [x] Bring from Rust 2015 to Rust 2021
  * [x] Migrate from `log` to `tracing`
  * [x] Migrate from `rustc-serialize` to `serde_json`


Discussion / hints:

Fix patch:
diff --git a/Justfile b/Justfile
index 58253166..09aa80a9 100644
--- a/Justfile
+++ b/Justfile
@@ -7,8 +7,8 @@ _default:
 ci-test:
 	#!/bin/bash -eux
 	just build-testbed
-	cargo llvm-cov --no-report nextest
-	cargo llvm-cov --no-report nextest --manifest-path crates/hring-hpack/Cargo.toml --features interop_tests --release
+	cargo nextest run --manifest-path crates/hring-hpack/Cargo.toml --features interop-tests --release
+	cargo llvm-cov --no-report nextest --profile ci
 	cargo llvm-cov --no-report run --manifest-path test-crates/hring-h2spec/Cargo.toml -- -j 'target/h2spec-junit.xml'
 	cargo llvm-cov report --lcov --output-path coverage.lcov
 	codecov
@@ -38,4 +38,4 @@ h2spec *args:
 	cargo run --manifest-path test-crates/hring-h2spec/Cargo.toml -- {{args}}
 
 check:
-	cargo clippy --all-targets
+	cargo clippy --all-targets --all-features
diff --git a/crates/hring-hpack/.travis.yml b/crates/hring-hpack/.travis.yml
deleted file mode 100644
index d2b8f3fa..00000000
--- a/crates/hring-hpack/.travis.yml
+++ /dev/null
@@ -1,33 +0,0 @@
-language: rust
-
-# necessary for `travis-cargo coveralls --no-sudo`
-addons:
-  apt:
-    packages:
-      - libcurl4-openssl-dev
-      - libelf-dev
-      - libdw-dev
-
-matrix:
-  include:
-    - rust: nightly
-    - rust: beta
-    - rust: stable
-
-# load travis-cargo
-before_script:
-  - |
-      pip install 'travis-cargo<0.2' --user &&
-      export PATH=$HOME/.local/bin:$PATH
-
-script: cargo test --features "interop_tests"
-
-sudo: false
-
-after_success:
-  - travis-cargo --only stable doc && travis-cargo --only stable doc-upload
-  - travis-cargo coveralls --no-sudo
-
-env:
-  global:
-    secure: HmpHD+2T5ZBPHcFfY4TVQtdjLsqENXIzn08WM4ZlE+XkQD8afToNN1dqKNm/jrFrCNkk4r6cU7PQFtuteJDTpeu/XbUb8eSmfmtECpxVJQfMDdgbzfEG5ZpNaU/nQLmU61/T7KLK9AsDbebX4sV+V5VfddXwJ0jt2bZlHd7DxOQ=
diff --git a/crates/hring-hpack/Cargo.toml b/crates/hring-hpack/Cargo.toml
index cc58f87d..7de3aedc 100644
--- a/crates/hring-hpack/Cargo.toml
+++ b/crates/hring-hpack/Cargo.toml
@@ -1,8 +1,8 @@
 [package]
-
 name = "hring-hpack"
 version = "0.3.0"
 authors = ["Marko Lalic <marko.lalic@gmail.com>", "Amos Wenger <amos@bearcove.net>"]
+edition = "2021"
 
 exclude = [
     "fixtures/*",
@@ -17,14 +17,14 @@ keywords = ["hpack", "http2", "encoding", "decoding"]
 documentation = "https://docs.rs/hpack-hring"
 
 [features]
+# these are really slow unless `cargo test --release` is used, so they're optional
+interop-tests = []
 
-# A feature defining some interop tests between the HPACK decoder and
-# headers encoded by other libraries. It is only ever used in test builds!
-interop_tests = ["rustc-serialize"]
+[dev-dependencies]
+serde = { version = "1.0.152", features = ["derive"] }
+serde_json = "1.0.91"
+hex = "0.4.3"
+thiserror = "1.0.38"
 
 [dependencies]
-log = "^0.3"
-
-[dependencies.rustc-serialize]
-version = "^0.3"
-optional = true
+tracing = "0.1"
diff --git a/crates/hring-hpack/appveyor.yml b/crates/hring-hpack/appveyor.yml
deleted file mode 100644
index c1ecb399..00000000
--- a/crates/hring-hpack/appveyor.yml
+++ /dev/null
@@ -1,21 +0,0 @@
-install:
-  - ps: |
-      if ($env:PLATFORM -eq "x86") {
-        Start-FileDownload 'https://static.rust-lang.org/dist/rust-nightly-i686-pc-windows-gnu.exe' -FileName rust.exe
-      } else {
-        Start-FileDownload 'https://static.rust-lang.org/dist/rust-nightly-x86_64-pc-windows-gnu.exe' -FileName rust.exe
-      }
-  - rust.exe /VERYSILENT /NORESTART /DIR="C:\Program Files\Rust"
-  - SET PATH=%PATH%;C:\Program Files\Rust\bin
-  - rustc -V
-  - cargo -V
-
-platform:
-  - x86
-  - x64
-
-build: false
-
-test_script:
-  - cargo test --verbose --features="interop_tests"
-  - cargo test --verbose
diff --git a/crates/hring-hpack/src/decoder.rs b/crates/hring-hpack/src/decoder.rs
index 88ac0c73..251e604b 100644
--- a/crates/hring-hpack/src/decoder.rs
+++ b/crates/hring-hpack/src/decoder.rs
@@ -48,6 +48,8 @@
 use std::borrow::Cow;
 use std::num::Wrapping;
 
+use tracing::{debug, info};
+
 use super::huffman::HuffmanDecoder;
 use super::huffman::HuffmanDecoderError;
 
@@ -1423,20 +1425,32 @@ mod tests {
 /// and some other encoder implementations, based on their results
 /// published at
 /// [http2jp/hpack-test-case](https://github.com/http2jp/hpack-test-case)
-#[cfg(feature = "interop_tests")]
 #[cfg(test)]
+#[cfg(feature = "interop-tests")]
 mod interop_tests {
-    use std::collections::HashMap;
-    use std::fs::{self, File};
-    use std::io::Read;
+    use std::fs;
     use std::path::{Path, PathBuf};
+    use std::{borrow::Cow, collections::HashMap};
 
-    use rustc_serialize::hex::FromHex;
-    use rustc_serialize::Decoder as JsonDecoder;
-    use rustc_serialize::{json, Decodable};
+    use serde::Deserialize;
+    use tracing::debug;
 
     use super::Decoder;
 
+    #[derive(Deserialize)]
+    struct RawTestStory<'a> {
+        cases: Vec<RawTestFixture<'a>>,
+    }
+
+    #[derive(Deserialize)]
+    struct RawTestFixture<'a> {
+        // The raw bytes of the message, as a hex-encoded string.
+        wire: Cow<'a, str>,
+
+        // The headers of the message: each HashMap has a single key-value pair
+        headers: Vec<HashMap<Cow<'a, str>, Cow<'a, str>>>,
+    }
+
     /// Defines the structure of a single part of a story file. We only care
     /// about the bytes and corresponding headers and ignore the rest.
     struct TestFixture {
@@ -1446,57 +1460,41 @@ mod interop_tests {
 
     /// Defines the structure corresponding to a full story file. We only
     /// care about the cases for now.
-    #[derive(RustcDecodable)]
     struct TestStory {
         cases: Vec<TestFixture>,
     }
 
-    /// A custom implementation of the `rustc_serialize::Decodable` trait for
-    /// `TestFixture`s. This is necessary for two reasons:
-    ///
-    ///  - The original story files store the raw bytes as a hex-encoded
-    ///    *string*, so we convert it to a `Vec<u8>` at parse time
-    ///  - The original story files store the list of headers as an array of
-    ///    objects, where each object has a single key. We convert this to a
-    ///    more natural representation of a `Vec` of two-tuples.
-    ///
-    /// For an example of the test story JSON structure check the
-    /// `test_story_parser_sanity_check` test function or one of the fixtures
-    /// in the directory `fixtures/hpack/interop`.
-    impl Decodable for TestFixture {
-        fn decode<D: JsonDecoder>(d: &mut D) -> Result<Self, D::Error> {
-            d.read_struct("root", 0, |d| {
-                Ok(TestFixture {
-                    wire_bytes: try!(d.read_struct_field("wire", 0, |d| {
-                        // Read the `wire` field...
-                        Decodable::decode(d).and_then(|res: String| {
-                            // If valid, parse out the octets from the String by
-                            // considering it a hex encoded byte sequence.
-                            Ok(res.from_hex().unwrap())
-                        })
-                    })),
-                    headers: try!(d.read_struct_field("headers", 0, |d| {
-                        // Read the `headers` field...
-                        d.read_seq(|d, len| {
-                            // ...since it's an array, we step into the sequence
-                            // and read each element.
-                            let mut ret: Vec<(Vec<u8>, Vec<u8>)> = Vec::new();
-                            for i in (0..len) {
-                                // Individual elements are encoded as a simple
-                                // JSON object with one key: value pair.
-                                let header: HashMap<String, String> =
-                                    try!(d.read_seq_elt(i, |d| Decodable::decode(d)));
-                                // We convert it to a tuple, which is a more
-                                // natural representation of headers.
-                                for (name, value) in header.into_iter() {
-                                    ret.push((name.as_bytes().to_vec(), value.as_bytes().to_vec()));
-                                }
-                            }
-                            Ok(ret)
+    #[derive(Debug, thiserror::Error)]
+    enum DecodeError {
+        #[error("Failed to parse hex-encoded bytes: {0}")]
+        FromHex(#[from] hex::FromHexError),
+    }
+
+    impl TryFrom<RawTestStory<'_>> for TestStory {
+        type Error = DecodeError;
+
+        fn try_from(raw_story: RawTestStory) -> Result<Self, Self::Error> {
+            let mut cases = Vec::with_capacity(raw_story.cases.len());
+            for raw_case in raw_story.cases {
+                let wire_bytes = hex::decode(raw_case.wire.as_bytes())?;
+
+                let headers: Vec<_> = raw_case
+                    .headers
+                    .into_iter()
+                    .flat_map(|h| {
+                        h.into_iter().map(|(k, v)| {
+                            (k.into_owned().into_bytes(), v.into_owned().into_bytes())
                         })
-                    })),
-                })
-            })
+                    })
+                    .collect();
+
+                cases.push(TestFixture {
+                    wire_bytes,
+                    headers,
+                });
+            }
+
+            Ok(TestStory { cases })
         }
     }
 
@@ -1504,7 +1502,7 @@ mod interop_tests {
     /// string. Sanity check for the `Decodable` implementation.
     #[test]
     fn test_story_parser_sanity_check() {
-        let raw_json = stringify!(
+        let raw_json = r#"
             {
               "cases": [
                 {
@@ -1546,9 +1544,10 @@ mod interop_tests {
               ],
               "draft": 9
             }
-        );
+        "#;
 
-        let decoded: TestStory = json::decode(raw_json).unwrap();
+        let decoded: RawTestStory = serde_json::from_str(raw_json).unwrap();
+        let decoded = TestStory::try_from(decoded).unwrap();
 
         assert_eq!(decoded.cases.len(), 2);
         assert_eq!(
@@ -1576,10 +1575,9 @@ mod interop_tests {
     fn test_story(story_file_name: PathBuf) {
         // Set up the story by parsing the given file
         let story: TestStory = {
-            let mut file = File::open(&story_file_name).unwrap();
-            let mut raw_story = String::new();
-            file.read_to_string(&mut raw_story).unwrap();
-            json::decode(&raw_story).unwrap()
+            let buf = std::fs::read_to_string(&story_file_name);
+            let raw_story: RawTestStory = serde_json::from_str(&buf.unwrap()).unwrap();
+            raw_story.try_into().unwrap()
         };
         // Set up the decoder
         let mut decoder = Decoder::new();
@@ -1597,7 +1595,7 @@ mod interop_tests {
     /// It calls the `test_story` function for each file found in the given
     /// directory.
     fn test_fixture_set(fixture_dir: &str) {
-        let files = fs::read_dir(&Path::new(fixture_dir)).unwrap();
+        let files = fs::read_dir(Path::new(fixture_dir)).unwrap();
 
         for fixture in files {
             let file_name = fixture.unwrap().path();
diff --git a/crates/hring-hpack/src/encoder.rs b/crates/hring-hpack/src/encoder.rs
index 5a84b08f..c16b0702 100644
--- a/crates/hring-hpack/src/encoder.rs
+++ b/crates/hring-hpack/src/encoder.rs
@@ -325,6 +325,8 @@ impl<'a> Encoder<'a> {
 
 #[cfg(test)]
 mod tests {
+    use tracing::debug;
+
     use super::encode_integer;
     use super::Encoder;
 
diff --git a/crates/hring-hpack/src/lib.rs b/crates/hring-hpack/src/lib.rs
index 6e91a6de..99556952 100644
--- a/crates/hring-hpack/src/lib.rs
+++ b/crates/hring-hpack/src/lib.rs
@@ -2,14 +2,11 @@
 //! performing the encoding and decoding of header sets, according to the
 //! HPACK spec.
 
-#[macro_use]
-extern crate log;
-#[cfg(feature = "interop_tests")]
-extern crate rustc_serialize;
-
 use std::collections::VecDeque;
 use std::fmt;
 
+use tracing::debug;
+
 // Re-export the main HPACK API entry points.
 pub use self::decoder::Decoder;
 pub use self::encoder::Encoder;


Test patch:
diff --git a/.config/nextest.toml b/.config/nextest.toml
index 5279ff1b..a40c6c9f 100644
--- a/.config/nextest.toml
+++ b/.config/nextest.toml
@@ -1,2 +1,5 @@
 [profile.default]
-slow-timeout = { period = "5s", terminate-after = 2 }
\ No newline at end of file
+slow-timeout = { period = "5s", terminate-after = 3 }
+
+[profile.ci]
+slow-timeout = { period = "5s", terminate-after = 6 }
\ No newline at end of file