assert_toml_snapshot produces invalid TOML when the toml library would generate an error.

[ilyagr]

What happened?

Here is a demo (using toml, maplit and insta {features=["serde"]} crates. Note how the first assert_toml_snaphot produces invalid TOML. The rest of the example is simply to provide some context of how the TOML library errors out and to confirm that JSON is generated correctly:

    use std::collections::BTreeMap;

    use maplit::btreemap;

    #[test]
    fn toml() {
        let m: BTreeMap<&str, Vec<Option<&str>>> = btreemap! {
            "a" => vec![Some("something"), None, Some("something else")]
        };
        // BUG!
        insta::assert_toml_snapshot!(m, 
        @r###"
        a = [
            'something'
        "###);
        
        // JSON can be generated fine
        insta::assert_json_snapshot!(m, 
        @r###"
        {
          "a": [
            "something",
            null,
            "something else"
          ]
        }
        "###);

       // The `toml` library generates an error in this case
        insta::assert_debug_snapshot!(toml::ser::to_string(&m), @r###"
        Err(
            Error {
                inner: UnsupportedNone,
            },
        )
        "###);
        
    }

Reproduction steps

No response

Insta Version

1.34.0

rustc Version

1.78.0-nightly (d44e3b95c 2024-02-09)

What did you expect?

I guess assert_toml_snapshot should have panicked.

[mitsuhiko]

This is an issue with the old version of toml that insta depends on. Upgrading to a newer one will unfortunately break all snapshots because the format of strings changed 'foo' => "foo". Not sure what is the best course of action here.

[ilyagr]

One possibility that comes to mind is to introduce a new feature to the crate along the lines of toml_updated. Then, new users can use it and old user can consciously upgrade. If there's ever a major version bump for insta, it can replace the current toml feature.

I'm not sure if it's worth the trouble, though.

[max-sixty]

The new feature is clever...

...though from someone who is not a maintainer (but a keen advocate and has contributed over the years) — a breaking change seems fairly reasonable. Cargo's dependency management means that it's a single commit; it's not like different contributors are clashing on different versions...

[mitsuhiko]

I wonder if the right course of action here is a major version of insta that bumps up these old libraries.

[ilyagr]

That sounds great to me, if you are willing. I am not sure whether or not this would inconvenience others.