Skip to content
This repository has been archived by the owner on Dec 17, 2024. It is now read-only.

Readable/Compact still give "must use Configure" error when serializing Map #34

Closed
sleepymurph opened this issue Apr 28, 2024 · 1 comment · Fixed by #35
Closed

Readable/Compact still give "must use Configure" error when serializing Map #34

sleepymurph opened this issue Apr 28, 2024 · 1 comment · Fixed by #35

Comments

@sleepymurph
Copy link

Hello,

I am loving this crate. Thank you! But I think I found an issue in the Readable/Compact wrappers.

I am building a custom data structure that acts like a Map of UUIDs to integers. UUIDs from the UUID crate serialize as strings when human readable and plain bytes when compact, so I am calling Configure::readable() to mark the test case. However, when running the test, I still get the error that tells me I have not marked the case:

Types which have different human-readable and compact representations must explicitly mark their test cases with serde_test::Configure

Minimum Working Example

[dependencies]
serde = { version = "1.0.198", features = ["derive"] }
uuid = { version = "1.8.0", features = ["v4", "serde"] }

[dev-dependencies]
serde_test = "1.0.176"
#[cfg(test)]
mod test_mwe {
    use std::collections::BTreeMap;

    use serde_test::{assert_tokens, Configure, Token};
    use uuid::{uuid, Uuid};

    /// Normal Uuid test shows difference between readable and compact.
    /// This test passes.
    #[test]
    fn test_uuid() {
        const ID_A: Uuid = uuid!("ae32aa4f-144f-44b1-a3cf-8a0f0b4ba44d");
        assert_tokens(
            &ID_A.readable(),  // <-- Test case is marked
            &[Token::String("ae32aa4f-144f-44b1-a3cf-8a0f0b4ba44d")],
        );
        assert_tokens(&ID_A.compact(), &[Token::Bytes(ID_A.as_bytes())]);
    }
    // || test test_mwe::test_uuid ... ok

    /// A test with a Uuid inside a map.
    /// Even though we do explicitly call `readable`, the test fails
    /// with an error telling us that we need to call `readable` or `compact`.
    #[test]
    fn test_map_of_uuid() {
        let mut vmap = BTreeMap::new();
        vmap.insert(uuid!("ae32aa4f-144f-44b1-a3cf-8a0f0b4ba44d"), 1 as u64);

        assert_tokens(
            &vmap.readable(),  // <-- Test case is marked
            &[
                Token::Map { len: Some(1) },
                Token::String("ae32aa4f-144f-44b1-a3cf-8a0f0b4ba44d"),
                Token::U64(1),
                Token::MapEnd,
            ],
        );
    }
    // || test test_mwe::test_map_of_uuid ... FAILED
    // || failures:
    // || ---- model::test_serde::test_map_of_uuid stdout ----
    // || thread 'model::test_serde::test_map_of_uuid' panicked at
    //        serde_test-1.0.176/src/ser.rs:307:9:
    // || Types which have different human-readable and compact representations
    //        must explicitly mark their test cases with `serde_test::Configure`
}

Possible Cause

I believe this has to do with the way Readable and Compact delegate to the unconfigured serde_test::ser::Serializer.
I followed the flow with gdb and saw this...

  1. Readable::serialize_entry() delegates to Serializer::serialize_entry()

    serde_test::configure::{impl#24}::serialize_entry<&mut serde_test::ser::Serializer, &uuid::Uuid, &u64>
        self.0.serialize_entry(key, &$wrapper(value))
  2. Serializer::serialize_entry() uses the default implementation in the serde::ser::SerializeMap trait, which calls its own serialize_key()

    serde::ser::SerializeMap::serialize_entry<&mut serde_test::ser::Serializer, &uuid::Uuid, serde_test::configure::Readable<&&u64>>
        tri!(self.serialize_key(key));
  3. Serializer::serialize_key() serializes the key, in this case a &Uuid

    serde_test::ser::{impl#6}::serialize_key<&uuid::Uuid>
        key.serialize(&mut **self)
  4. We make a brief stop in the generic serde crate's deref_impl macro,
    I think to dereference the &Uuid impl to get to the Uuid impl.

    serde::ser::impls::{impl#107}::serialize<uuid::Uuid, &mut serde_test::ser::Serializer>
        (**self).serialize(serializer)
  5. The serialize() function for the Uuid checks is_human_readable() on the original Serializer, not the Readable wrapper.

    uuid::external::serde_support::{impl#0}::serialize<&mut serde_test::ser::Serializer>
        if serializer.is_human_readable() {
  6. Serializer::is_human_readable() panics.

    serde_test::ser::{impl#1}::is_human_readable
        panic!(

What do you think?

Thanks again!
-Murph

@dtolnay
Copy link
Contributor

dtolnay commented Aug 5, 2024

Fixed in serde_test 1.0.177.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
2 participants