insta snapshot testing generation for postcard-schema

[cramt]

fixes #204

an opt-in flag called [postcard(snapshot)] to opt into having compile time generated snapshotting tests for your schemas. For now this explicitly fails at compile-time if used on a struct with non-lifetime generics

[netlify]

✅ Deploy Preview for cute-starship-2d9c9b canceled.

Name	Link
🔨 Latest commit	4194677
🔍 Latest deploy log	https://app.netlify.com/projects/cute-starship-2d9c9b/deploys/688641dc93eea900084166a9

[jamesmunns]

Interesting! I hadn't considered the limitation on generics. I suppose the only other option would be to instead generate a function that could be called in a test context, but isn't a top level "test" itself.

On the upside: you could support generics, because it could be done on each instantiation. On the downside: users don't just get the check for free.

I wonder if for now we should generate a failing test for generic types, so we don't mislead/confuse folks? I'm open to your thoughts here!

Also, once we answer the stuff above, it would be good to port this to postcard-schema-ng as well.

Thank you for working on this!

[cramt]

So since this is opt-in (and therefore the user probably explicitly decided "i want snapshot testing") it does make sense in my book to make a failing test.

If we have a simple case with no bounds (besides for Schema)

#[derive(Schema)]
struct Foo<T: Schema> {
     x: T,
     y: usize,
}

we could just have the test be on Foo<()> but obviously this solution only works for situations where the generics doesnt have any bounds.

A simple but maybe bad solution generics with bounds could be to just require that generics be defaulted or we generate a failing tests.

[jamesmunns]

Yeah, my only caveat is "if one of your deps enabled the insta feature, but you want generics, then you're kind of hosed unless you patch your dep".

It would be nice to have something like #[postcard(snapshot = skip)] or so for a type (not a field), like:

#[derive(Schema)]
#[postcard(snapshot = skip)]
struct Foo<T: Schema> {
     x: T,
     y: usize,
}

or maybe just a second derive, like:

#[derive(SchemaNoSnapshot)]
struct Foo<T: Schema> {
     x: T,
     y: usize,
}

But that's kinda weird, and you might still get in the case where one dep has generics and one dep enables insta.

Or maybe just an env var like POSTCARD_SCHEMA_SKIP_GENERIC_SNAPSHOTS that make the tests pass/skip?

I don't have a great way to solve this :/. I appreciate you working on this, and I don't want you to get discouraged! I have not thought this all the way through, so we definitely still need to think around the edge cases of this a bit so this doesn't upset any users.

[jamesmunns]

Maybe the right choice is having a totally separate derive for snapshot testing to make it really explicitly opt-in? e.g. #[derive(SchemaSnapshot)]?

[cramt]

Yeah, my only caveat is "if one of your deps enabled the insta feature, but you want generics, then you're kind of hosed unless you patch your dep".

I mean if you as a library author had insta enabled as a default feature that would be kinda silly and rude ngl 😅, but yeah having explicit opt-in would probably be best. Though imo doing #[postcard(snapshot)] would probably be better than a second derive because then you can accidentally make something derive SchemaSnapshot without deriving Schema. And then just ending with a compile error if you try to do #[postcard(snapshot)] on something with a generic bound more specific than Schema

[max-heller]

Though imo doing #[postcard(snapshot)] would probably be better than a second derive because then you can accidentally make something derive SchemaSnapshot without deriving Schema. And then just ending with a compile error if you try to do #[postcard(snapshot)] on something with a generic bound more specific than Schema

Agreed that an opt-in #[postcard(snapshot)] makes sense, it might not be desirable to generate tests for all types implementing Schema even if #[derive(Schema)] could.

For generic types, what about something like #[postcard(snapshot(TYPE))]?

#[derive(Schema)]
#[postcard(snapshot(Foo<()>))]
struct Foo<T: Schema> {
     x: T,
     y: usize,
}

Bikeshed: does the name of the attribute need more context? #[postcard(schema(snapshot))] or #[postcard(schema_snapshot)] or ...

[jamesmunns]

I think shipping a first version of this (like postcard(snapshot)) that doesn't support generics is fine, as long as we could possibly support it in the future.

I would think in most cased (randomly guessing tho) we'd more often have generic types like:

// in some library/dep somewhere...
#[derive(Schema)]
struct Foo<T: Schema> {
     x: T,
     y: usize,
}

// actual "leaf" type
#[derive(Schema)]
#[postcard(snapshot)]
struct ActualType {
    foo: Foo<u64>,
}

Idk tho! I'm not sure how easy it would be to do:

#[postcard(snapshot)]
type MyFoo = Foo<u64>;

That probably takes us out of derive territory and into general proc macros? Not my strong field 😅

[cramt]

since we are making it opt-in should we maybe not have it be a feature? not really sure what the convention on features like this is when dealing mostly in crates that focus a lot on an embedded audience

[jamesmunns]

Agreed that if we don't make it automatic, we don't need it to be a feature (unless we pull in a bunch more deps or something to support it).

[cramt]

also maybe some biksheeding is needed here, currently the test is just named insta_{struct_name}, i feel like this is a terrible name for the generated tests, but not sure i cant think of a better one

[jamesmunns]

postcard_snapshot_{struct_name}? I'm honestly fine with whatever color you want the bikeshed to be if you're the one doing the painting :)