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: allow testing for functional options #1023

Merged
merged 3 commits into from May 9, 2023

Conversation

nbaztec
Copy link
Contributor

@nbaztec nbaztec commented Nov 9, 2020

Summary

This PR fixes #997

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

c := client.New(client.WithTimeout(), client.WithLabel("bar"))
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}
}

Changes

Adds a new argument type FunctionalOptions

Motivation

This wasn't possible before.

Related issues

Closes #997 #1006

alameddinc
alameddinc previously approved these changes Jul 26, 2022
Copy link

@alameddinc alameddinc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think It should be inserted into the mock library 👍

Copy link
Contributor

@dillonstreator dillonstreator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nbaztec thoughts on these changes to simplify the signature?

mock/mock.go Outdated Show resolved Hide resolved
mock/mock.go Outdated Show resolved Hide resolved
mock/mock.go Outdated Show resolved Hide resolved
mock/mock.go Outdated Show resolved Hide resolved
mock/mock_test.go Outdated Show resolved Hide resolved
mock/mock_test.go Show resolved Hide resolved
@dillonstreator
Copy link
Contributor

@boyan-soubachov can you please provide feedback on this PR?

Co-authored-by: dillonstreator <dillonstreator@gmail.com>
@nbaztec
Copy link
Contributor Author

nbaztec commented Jan 20, 2023

@dillonstreator The suggestions look good, I've incorporated them.
I was a bit hesitant at first if there could arise a situation during a refactor/upgrade, where the first parameter could be an incorrect type in the assertion, thereby using an incorrect type - but that seems like a non-issue, since the same can happen when specifying a manual string.

Copy link
Collaborator

@boyan-soubachov boyan-soubachov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you :)

@boyan-soubachov boyan-soubachov merged commit b3106d7 into stretchr:master May 9, 2023
@dolmen dolmen added pkg-mock Any issues related to Mock mock.ArgumentMatcher About matching arguments in mock labels Mar 22, 2024
@dolmen dolmen changed the title allow testing for functional options mock: allow testing for functional options Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement input needed mock.ArgumentMatcher About matching arguments in mock pkg-mock Any issues related to Mock
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mock Matcher for functional options
5 participants