gkampitakis / go-snaps

Jest-like snapshot testing in Go 📸
https://pkg.go.dev/github.com/gkampitakis/go-snaps
MIT License
158 stars 6 forks source link

Add support for exclusion of field via field tag. #51

Closed NicklasWallgren closed 7 months ago

NicklasWallgren commented 1 year ago

With these changes, one can add a struct field tag go-snaps:"-" to ignore pretty printing a particular field.

type testFieldTag struct {
    x, y int
    z string `go-snaps:"-"`
}

It's a good option until support for matchers has been released.

gkampitakis commented 1 year ago

Hey, thanks a lot for the pr 😊 . Will have a look at it as soon as possible

gkampitakis commented 1 year ago

Sorry for the delay, first of all, thanks again for your contribution and for your interest in the library.

I have some concerns with this solution. First, it's pretty clever using the tags, I never thought about it, but I am worried people are already making the structs really "crowded" with tags, not sure they would want to add to that long list another one that would be for tests only. Secondly, i am not sure I feel comfortable vendoring another module for functionality that matchJSON partially can already support. ( and possibly in the future creating a matchStruct() that can leverage the same matchers )

I am not saying no I am just sharing thoughts and asking for feedback :sweat_smile: potentially on the matchJSON functionality as well.

NicklasWallgren commented 1 year ago

Thanks for the reply!

I wasn't aware that you were working on adding support for property matchers when I created this PR. I noticed https://github.com/gkampitakis/go-snaps/issues/18, and thought that I was forced to implement my own snapshot-library, with support for matchers.

I'm really glad you started working on matchJSON, since the current implementation doesn't take dynamic properties into account.

I'm not to comfortable about the vendoring either. There is actually an open PR on kr/pretty with similar support, https://github.com/kr/pretty/pull/51.

gkampitakis commented 1 year ago

Hey :wave:, just to keep you on the loop haven't forgot about this pr. Just waiting if there is some interest on merging the pr you shared on pretty

gkampitakis commented 7 months ago

Closing this. If there is interest from pretty to imlpement it, I am happy to document this.