chore: Guard testing functions properly (#2264)

* chore: Guard testing functions properly

We have a bunch of functions that in their doc comments say that they are
for testing purposes only. This commit adds guards to those functions
to make sure they are not used in production code.

* More

neqo-common/Cargo.toml

@@ -37,6 +37,7 @@ regex = { workspace = true }
 bench = ["test-fixture/bench"]
 build-fuzzing-corpus = ["hex"]
 ci = []
+test-fixture = []
 
 [lib]
 # See https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options

neqo-common/src/codec.rs

@@ -44,19 +44,23 @@ impl<'a> Decoder<'a> {
     }
 
     /// Skip helper that panics if `n` is `None` or not able to fit in `usize`.
+    /// Only use this for tests because we panic rather than reporting a result.
+    #[cfg(any(test, feature = "test-fixture"))]
     fn skip_inner(&mut self, n: Option<u64>) {
         self.skip(usize::try_from(n.expect("invalid length")).unwrap());
     }
 
     /// Skip a vector.  Panics if there isn't enough space.
     /// Only use this for tests because we panic rather than reporting a result.
+    #[cfg(any(test, feature = "test-fixture"))]
     pub fn skip_vec(&mut self, n: usize) {
         let len = self.decode_uint(n);
         self.skip_inner(len);
     }
 
     /// Skip a variable length vector.  Panics if there isn't enough space.
     /// Only use this for tests because we panic rather than reporting a result.
+    #[cfg(any(test, feature = "test-fixture"))]
     pub fn skip_vvec(&mut self) {
         let len = self.decode_varint();
         self.skip_inner(len);
@@ -272,6 +276,7 @@ impl Encoder {
     /// # Panics
     ///
     /// When `s` contains non-hex values or an odd number of values.
+    #[cfg(any(test, feature = "test-fixture"))]
     #[must_use]
     pub fn from_hex(s: impl AsRef<str>) -> Self {
         let s = s.as_ref();

neqo-qpack/src/encoder_instructions.rs

@@ -18,17 +18,33 @@ use crate::{
     Res,
 };
 
-// The encoder only uses InsertWithNameLiteral, therefore clippy is complaining about dead_code.
-// We may decide to use othe instruction in the future.
-// All instructions are used for testing, therefore they are defined.
-#[allow(dead_code)]
+// The encoder only uses InsertWithNameLiteral.
+// All instructions are used for testing, therefore they are guarded with `#[cfg(test)]`.
 #[derive(Debug, PartialEq, Eq)]
 pub enum EncoderInstruction<'a> {
-    Capacity { value: u64 },
-    InsertWithNameRefStatic { index: u64, value: &'a [u8] },
-    InsertWithNameRefDynamic { index: u64, value: &'a [u8] },
-    InsertWithNameLiteral { name: &'a [u8], value: &'a [u8] },
-    Duplicate { index: u64 },
+    Capacity {
+        value: u64,
+    },
+    #[cfg(test)]
+    InsertWithNameRefStatic {
+        index: u64,
+        value: &'a [u8],
+    },
+    #[cfg(test)]
+    InsertWithNameRefDynamic {
+        index: u64,
+        value: &'a [u8],
+    },
+    InsertWithNameLiteral {
+        name: &'a [u8],
+        value: &'a [u8],
+    },
+    #[cfg(test)]
+    Duplicate {
+        index: u64,
+    },
+    #[cfg(test)]
+    #[allow(dead_code)]
     NoInstruction,
 }
 
@@ -38,10 +54,12 @@ impl EncoderInstruction<'_> {
             Self::Capacity { value } => {
                 enc.encode_prefixed_encoded_int(ENCODER_CAPACITY, *value);
             }
+            #[cfg(test)]
             Self::InsertWithNameRefStatic { index, value } => {
                 enc.encode_prefixed_encoded_int(ENCODER_INSERT_WITH_NAME_REF_STATIC, *index);
                 enc.encode_literal(use_huffman, NO_PREFIX, value);
             }
+            #[cfg(test)]
             Self::InsertWithNameRefDynamic { index, value } => {
                 enc.encode_prefixed_encoded_int(ENCODER_INSERT_WITH_NAME_REF_DYNAMIC, *index);
                 enc.encode_literal(use_huffman, NO_PREFIX, value);
@@ -50,9 +68,11 @@ impl EncoderInstruction<'_> {
                 enc.encode_literal(use_huffman, ENCODER_INSERT_WITH_NAME_LITERAL, name);
                 enc.encode_literal(use_huffman, NO_PREFIX, value);
             }
+            #[cfg(test)]
             Self::Duplicate { index } => {
                 enc.encode_prefixed_encoded_int(ENCODER_DUPLICATE, *index);
             }
+            #[cfg(test)]
             Self::NoInstruction => {}
         }
     }
@@ -81,12 +101,14 @@ impl<'a> From<&'a EncoderInstruction<'a>> for DecodedEncoderInstruction {
     fn from(inst: &'a EncoderInstruction) -> Self {
         match inst {
             EncoderInstruction::Capacity { value } => Self::Capacity { value: *value },
+            #[cfg(test)]
             EncoderInstruction::InsertWithNameRefStatic { index, value } => {
                 Self::InsertWithNameRefStatic {
                     index: *index,
                     value: value.to_vec(),
                 }
             }
+            #[cfg(test)]
             EncoderInstruction::InsertWithNameRefDynamic { index, value } => {
                 Self::InsertWithNameRefDynamic {
                     index: *index,
@@ -99,7 +121,9 @@ impl<'a> From<&'a EncoderInstruction<'a>> for DecodedEncoderInstruction {
                     value: value.to_vec(),
                 }
             }
+            #[cfg(test)]
             EncoderInstruction::Duplicate { index } => Self::Duplicate { index: *index },
+            #[cfg(test)]
             EncoderInstruction::NoInstruction => Self::NoInstruction,
         }
     }

test-fixture/Cargo.toml

@@ -18,7 +18,7 @@ workspace = true
 [dependencies]
 # Checked against https://searchfox.org/mozilla-central/source/Cargo.lock 2024-11-11
 log = { workspace = true }
-neqo-common = { path = "../neqo-common" }
+neqo-common = { path = "../neqo-common", features = ["test-fixture"] }
 neqo-crypto = { path = "../neqo-crypto" }
 neqo-http3 = { path = "../neqo-http3" }
 neqo-transport = { path = "../neqo-transport" }