Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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
Member

dtolnay commented Aug 5, 2024

Fixed in serde_test 1.0.177.

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