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

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

Open
josephwoodward opened this issue Oct 18, 2021 · 17 comments
Open

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

josephwoodward opened this issue Oct 18, 2021 · 17 comments

Comments

@josephwoodward
Copy link
Contributor

josephwoodward commented Oct 18, 2021

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
Copy link
Contributor

hjwk commented Nov 22, 2021

It is not currently possible

@josephwoodward
Copy link
Contributor Author

@hjwk Okay, thanks!

@josephwoodward
Copy link
Contributor Author

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

@hjwk
Copy link
Contributor

hjwk commented Mar 31, 2022

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

@emilybache
Copy link
Contributor

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
Copy link
Contributor Author

josephwoodward commented Apr 1, 2022

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

@josephwoodward
Copy link
Contributor Author

josephwoodward commented Apr 3, 2022

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
Copy link
Contributor

objarni commented Apr 3, 2022

@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
Copy link
Contributor Author

josephwoodward commented Apr 4, 2022

@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
Copy link
Contributor Author

josephwoodward commented Apr 5, 2022

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
Copy link
Contributor

objarni commented Apr 5, 2022

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

josephwoodward commented Apr 6, 2022

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
Copy link
Contributor

objarni commented Apr 6, 2022

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
Copy link
Contributor Author

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
Copy link
Contributor

objarni commented Apr 7, 2022

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

@josephwoodward
Copy link
Contributor Author

josephwoodward commented Apr 11, 2022

@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 #37

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

No branches or pull requests

4 participants