approvals / go-approval-tests

Apache License 2.0
86 stars 21 forks source link

Is it possible to exclude parts of an output when testing? #31

Open josephwoodward opened 2 years ago

josephwoodward commented 2 years ago

Hi,

If approval testing a data structure that contains dynamic content (such as a date or timestamp), is it possible to ignore it when running an approval test (similar to scrubbers in Shouldly - see here?

Many thanks!

hjwk commented 2 years ago

It is not currently possible

josephwoodward commented 2 years ago

@hjwk Okay, thanks!

josephwoodward commented 2 years ago

@hjwk Would you be open to me creating a PR to add such a functionality?

hjwk commented 2 years ago

I am not a maintainer, I would rather ask @emilybache or @objarni

emilybache commented 2 years ago

I'd be happy to get a pull request for Scrubbers. Ideally it would work in a similar way to the C++ Scrubbers:

https://approvaltestscpp.readthedocs.io/en/latest/generated_docs/explanations/Scrubbers.html

so an option to the verify function.

josephwoodward commented 2 years ago

@emilybache Great , I’ll take a look at the C++ Scrubbers and post some ideas.

josephwoodward commented 2 years ago

Hi @emilybache, what about introducing an API such as below. It seems like it closely aligns with the C++ scrubbers API whilst still promoting idiomatic Go.

scrubber, _ := regexp.Compile("\\d{10}$")
approvals.VerifyJSONStruct(t, json, approvals.WithScrubber(scrubber))
objarni commented 2 years ago

@JosephWoodward I like that you are looking at scrubbers for the Golang approval testing library implementation!

I do have a concern with your suggested API, in that it does not (seem) to use an Options fluent API [1] which is the 'creme de la creme' pattern used in the C++, Python, Java etc libraries. Or is it? Is the WithScrubber method returning an Option structure?

Note: Options-fluent-syntax does make adding the first option a little trickier than just adding a scrubber parameter to Verify* calls, but that is payed back with 2nd, 3rd and so on big time! See rationale described here [2].

If you'd like, we could pair-program this?

[1] See example here https://approvaltestscpp.readthedocs.io/en/latest/generated_docs/Options.html [2] Options rationale https://approvaltestscpp.readthedocs.io/en/latest/generated_docs/explanations/WhyWeAreConvertingToOptions.html

josephwoodward commented 2 years ago

@objarni @emilybache I see, so options can be applied globally like so, and also at an assertion by assertion instance like so:

scrubber, _ := regexp.Compile("\\d{10}$")
approvals.VerifyJSONStruct(t, json, approvals.WithScrubber(scrubber))

I notice there are approvals.UseReporter(...) for the global configuration though, is that the agreed way forward for global configuration in go-approval-tests? If so, is it just the assertion level options that's left to do? (as per @emilybache's comment above).

I do have a concern with your suggested API, in that it does not (seem) to use an Options fluent API [1] which is the 'creme de la creme' pattern used in the C++, Python, Java etc libraries. Or is it? Is the WithScrubber method returning an Option structure?

Yes, the options returns an option so can be chained. Perhaps I'll put together a better simplified draft PR before I get too invested. It'd be good to reduce the scope of the change to perhaps just a scrubber.

josephwoodward commented 2 years ago

For completeness, I noticed there are also some pre-made scrubbers that the API would need to support at some point?

Another option might be a more extensible (and idiomatic Go API) such as this:

scrubber, _ := regexp.Compile("\\d{10}$")

opts := approvals.WithOptions(
    approvals.WithScrubber(scrubber),
)

approvals.VerifyJSONStruct(t, json, opts)

You can could extend the options to include pre-made scrubbers, like so:

scrubber, _ := regexp.Compile("\\d{10}$")

opts := approvals.WithOptions(
    approvals.WithReporter(),
    approvals.WithGUIDScrubber(),
    approvals.WithScrubber(scrubber))

approvals.VerifyJSONStruct(t, json, opts)
objarni commented 2 years ago

Great progress.

I would have renamed WithOptions to Options.

I'm also curious why the options are listed instead of dotted together? The syntax in other languages is fluent style:

opts := Options().withReporter(r).withScrubber(s)

.. and so on.

josephwoodward commented 2 years ago

@objarni I was just exploring some other more idiomatic Go approaches (as I know method chaining is a contentious point in the Go community, and not all languages are equal). I'll take a look at how some Go libraries construct their fluent APIs and add a fluent version this evening.

josephwoodward commented 2 years ago

I've started on a draft PR to add support for scrubbers using a similar API to that of the other languages.

If you'd like, we could pair-program this?

More than happy to pair on bits if you like, it's always nice collaborating with others :)

objarni commented 2 years ago

Looking great!

The MVP would include the regex scrubber I guess? I'd focus on that and do the reporters, guid scrubber etc on separate PRs.

Especially if that would be enough to get you going in the application you're testing!

josephwoodward commented 2 years ago

The MVP would include the regex scrubber I guess? I'd focus on that and do the reporters, guid scrubber etc on separate PRs.

Yeah, the plan is to add support for regex scrubbers first, others can be added over time once the foundation exists.

objarni commented 2 years ago

Missed the invitation to pair program-yes we could do that! Sending you a private mail to setup a time.

josephwoodward commented 2 years ago

@emilybache @objarni I think I have the pull request ready for review, it'd be great if you could review it when you have time and give some feedback on anything I may have missed. I've added scrubbers to anything whose underlying implementation invoked VerifyString.

The pull request can be found here https://github.com/approvals/go-approval-tests/pull/37