editoast: derive: setup insta to snapshot test macros

Allows to easily test macro expansion, track changes, improves
debuggability and ease review.

HashMaps have been eliminated from `Model`'s internals to have a
deterministic expansion. Otherwise snapshots would be flaky.

Signed-off-by: Leo Valais <[email protected]>

Author: leovalais

.github/workflows/build.yml

@@ -589,6 +589,7 @@ jobs:
           docker load --input ./osrd-editoast-test.tar
 
       - name: Execute tests within container
+        # snapshot testing library `insta` requires CI=true
         run: |
           docker run --rm --net=host \
             -v $PWD/docker/init_db.sql:/init_db.sql \
@@ -597,6 +598,7 @@ jobs:
 
           docker run --name=editoast-test --net=host -v $PWD/output:/output \
             -e DATABASE_URL="postgres://osrd:password@localhost:5432/osrd" \
+            -e CI="true" \
             ${{ fromJSON(needs.build.outputs.stable_tags).editoast-test }} \
             /bin/sh -c "diesel migration run --locked-schema && RUST_BACKTRACE=1 cargo test --workspace --verbose -- --test-threads=4 && grcov . --binary-path ./target/debug/ -s . -t lcov --branch --ignore-not-existing --ignore "/*" -o /output/lcov.info"
 

editoast/Cargo.lock

@@ -8,7 +8,9 @@ edition.workspace = true
 
 [dependencies]
 darling = "0.20"
+insta = "1.41"
 paste.workspace = true
+prettyplease = "0.2"
 proc-macro2 = "1.0"
 quote = "1.0"
 serde_json.workspace = true

editoast/editoast_derive/Cargo.toml

@@ -235,3 +235,26 @@ fn extract_type(ty: &syn::Type) -> Option<String> {
         _ => None,
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_construction() {
+        crate::assert_macro_expansion!(
+            expand_editoast_error,
+            syn::parse_quote! {
+                #[derive(EditoastError)]
+                #[editoast_error(base_id = "infra", default_status = 500)]
+                pub enum InfraApiError {
+                    #[editoast_error(status = 404, no_context)]
+                    NotFound { infra_id: i64 },
+                    #[editoast_error(status = 400)]
+                    BadRequest { message: String },
+                    InternalError,
+                }
+            }
+        );
+    }
+}

editoast/editoast_derive/src/error.rs

@@ -237,3 +237,37 @@ pub fn model(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
         .unwrap_or_else(darling::Error::write_errors)
         .into()
 }
+
+#[cfg(test)]
+pub(crate) fn pretty_tokens(tokens: &proc_macro2::TokenStream) -> String {
+    let file = syn::parse_file(tokens.to_string().as_str()).unwrap();
+    prettyplease::unparse(&file)
+}
+
+// Extra allows because of some false negative lints, not sure why.
+// Maybe because editoast_derive is a proc-macro crate?
+
+#[cfg(test)]
+#[allow(unused_macros)]
+macro_rules! assert_macro_expansion {
+    ($expansion:path, $derive_input:expr) => {
+        let input: syn::DeriveInput = $derive_input;
+        let source = crate::pretty_tokens(&<syn::DeriveInput as quote::ToTokens>::to_token_stream(&input));
+        let expansion = $expansion(&input).expect("macro should expand faultlessly");
+        let expected = crate::pretty_tokens(&expansion);
+
+        // HACK: sadly insta doesn't let us print multiline strings in the scnapshot description
+        // or info sections. So we have to incorporate the source input into the snapshot content
+        // in order to keep it pretty printed and next to its expansion.
+        insta::with_settings!({
+            omit_expression => true,
+        }, {
+            let sep = "-".repeat(77);
+            insta::assert_snapshot!(format!("// Source\n// {sep}\n\n{source}\n// Macro expansion\n// {sep}\n\n{expected}"));
+        });
+    };
+}
+
+#[cfg(test)]
+#[allow(unused_imports)]
+use assert_macro_expansion;

editoast/editoast_derive/src/lib.rs

@@ -87,21 +87,27 @@ pub fn model(input: &DeriveInput) -> Result<TokenStream> {
 }
 
 #[cfg(test)]
-#[test]
-fn test_construction() {
-    let input = syn::parse_quote! {
-        #[derive(Clone, Model)]
-        #[model(table = editoast_models::tables::osrd_infra_document)]
-        #[model(row(type_name = "DocumentRow", derive(Debug)))]
-        #[model(changeset(type_name = "DocumentChangeset", public, derive(Debug)))] // fields are public
-        #[model(gen(ops = crud, batch_ops = crud, list))]
-        struct Document {
-            #[model(column = "id", preferred, primary)]
-            id_: i64,
-            #[model(identifier, json)]
-            content_type: String,
-            data: Vec<u8>,
-        }
-    };
-    let _ = model(&input).expect("should generate");
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_construction() {
+        crate::assert_macro_expansion!(
+            model,
+            syn::parse_quote! {
+                #[derive(Clone, Model)]
+                #[model(table = editoast_models::tables::osrd_infra_document)]
+                #[model(row(type_name = "DocumentRow", derive(Debug)))]
+                #[model(changeset(type_name = "DocumentChangeset", public, derive(Debug)))] // fields are public
+                #[model(gen(ops = crud, batch_ops = crud, list))]
+                struct Document {
+                    #[model(column = "id", preferred, primary)]
+                    id_: i64,
+                    #[model(identifier, json)]
+                    content_type: String,
+                    data: Vec<u8>,
+                }
+            }
+        );
+    }
 }

editoast/editoast_derive/src/model.rs

@@ -1,5 +1,5 @@
 use std::{
-    collections::HashSet,
+    collections::BTreeSet,
     ops::{Deref, DerefMut},
 };
 
@@ -19,9 +19,9 @@ pub(crate) struct ModelConfig {
     pub(crate) fields: Fields,
     pub(crate) row: GeneratedTypeArgs,
     pub(crate) changeset: GeneratedTypeArgs,
-    pub(crate) identifiers: HashSet<Identifier>, // identifiers ⊆ fields
-    pub(crate) preferred_identifier: Identifier, // preferred_identifier ∈ identifiers
-    pub(crate) primary_identifier: Identifier,   // primary_identifier ∈ identifiers
+    pub(crate) identifiers: BTreeSet<Identifier>, // identifiers ⊆ fields
+    pub(crate) preferred_identifier: Identifier,  // preferred_identifier ∈ identifiers
+    pub(crate) primary_identifier: Identifier,    // primary_identifier ∈ identifiers
     pub(crate) impl_plan: ImplPlan,
 }
 

editoast/editoast_derive/src/model/config.rs

@@ -85,6 +85,30 @@ impl darling::FromMeta for RawIdentifier {
     }
 }
 
+impl PartialOrd for RawIdentifier {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for RawIdentifier {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        self.get_idents().cmp(&other.get_idents())
+    }
+}
+
+impl PartialOrd for Identifier {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.raw.cmp(&other.raw))
+    }
+}
+
+impl Ord for Identifier {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        self.raw.cmp(&other.raw)
+    }
+}
+
 fn extract_ident_of_path(path: &syn::Path) -> darling::Result<syn::Ident> {
     let mut segments = path.segments.iter();
     let first = segments.next().unwrap();