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

Mock Matcher for functional options #997

Closed
nbaztec opened this issue Sep 5, 2020 · 3 comments · Fixed by #1023
Closed

Mock Matcher for functional options #997

nbaztec opened this issue Sep 5, 2020 · 3 comments · Fixed by #1023

Comments

@nbaztec
Copy link
Contributor

nbaztec commented Sep 5, 2020

Hi,

In most go libraries nowadays (etcd, kubernets, aws, etc.) the functional options pattern is being used:

c := client.New(client.WithTimeout(), client.WithLabel("bar"))

The mocks for these functional options are unfortunately unable to match since the option usually is implemented so:

type opt struct {
  timeout bool
  label string
}

type ClientOption func(*opt)

func WithTimeout() ClientOption {
  return func(o *opt) {
    o.timeout = true
  }
}

func WithLabel(v string) ClientOption {
  return func(o *opt) {
    o.label = v
  }
}

func (c *client) New(opts ...ClientOption) *someClient {
  options := &opt{}
  for _, f := range opts {
    f(options)
  }

  return &someClient{options.timeout, options.label}
}

testify.Mock matcher will not match x := WithTimeout() with y := WithTimeout() since these are actually distinct functions. Using mock.AnythingOfType defeats the purpose since these options are usually significant and even critical to behavior.

As a workaround currently, I'm using reflection and the Call.Run() (alt. mock.MatchedBy) function to match these opts and log a proper diff (different options, different type, wrong values, etc.). I thought about it and it would've been ideal if we can introduce a matcher type mock.OptionTypeArgument or something since the MatchedBy func is inadequate for printing meaningful diffs as it will only true/false info of the failure scenario.

Is this something that the library could/should support? If yes, I can open a PR to elicit the approach.

@boyan-soubachov
Copy link
Collaborator

We're happy to accept PRs and help you get them reviewed :)

Out of curiosity, does the https://github.com/vektra/mockery package work in your use case?

@nbaztec
Copy link
Contributor Author

nbaztec commented Sep 8, 2020

Thanks for pointing to mockery, but I think it is inherently hard to test functional options, since the values are themselves (inner) functions and thus the need for reflection and some runtime magic :)

I'm however interested in what would be a reasonable approach to asserting these arguments (provided a function to test equality exists) - either a new Argument type so a new path can be asserted here or perhaps if MatchedBy func definition allowed to return the actual and expected to be printed (then no further integration is necessary).

I'll try to submit a PR to covey my intent (after the next fortnight).

@nbaztec
Copy link
Contributor Author

nbaztec commented Nov 9, 2020

Sorry for getting back a bit late. Filed #1023 to better demonstrate my idea.

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

Successfully merging a pull request may close this issue.

2 participants