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: Conditionally toggle mock Call #1605

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

piotrpersona
Copy link

@piotrpersona piotrpersona commented May 30, 2024

Summary

Toggle on/off Unset method with an option.

Changes

  • New type UnsetOption was defined
  • New function that return UnsetOption were defined: WithUnsetToggle, WithUnsetEnabled, WithUnsetDisabled
  • Unset method will accept variadic list of UnsetOptions to be called, which makes the new change entirely backward-compatible
  • By default Unset will be enabled, which is also backward-compatible

Motivation

Consider the following example test scenario:

The goal is to test http server API. The test may set up a local http server, database and mocks to 3rd party services. Since the tests are calling a mocked server, it may be useful to use mock EXPECT() calls. Test structure:

  1. Setup local server, database and mocks
  2. Mock 3rd party service calls
  3. Send request to local server
  4. Assert response
  5. Verify side effects - execute database query

As an enhancement, such tests can easily be called against deployed application, by simply replacing server and database URLs. The only catch are mocked calls, which won't be executed, since the request is sent to a remote server.

In such case, it may be possible to call Unset() method with WithUnsetToggle option, which value is set based on the current environment. For example:

func Env() mock.UnsetOption {
    if os.Getenv("local") {
        return mock.WithUnsetToggle(false)
    }
    return mock.WithUnsetToggle(true)
}

// In test ...
func TestAPI(t *testing.T) {
    // setup server, database and mocks
    mock.Call3rdParty(ctx, argument).Return(nil).Unset(Env())

    response := client.Post(request)
    // assertions and database query
}

In the above example, if environment is "local" Unset will be disabled, meaning that all mocks will execute.
If environment is other than "local" the Unset will be enabled, meaning that mocks will not be called.

This allows to run the same suite of tests regardless of environment and setup.

Alternative approach

Edit: I also implemened Toggle method, that is way simpler and may be used in a similar way. I am looking forward to your feedback.

func Env() bool {
    return os.Getenv("ENV") == "local"
}
// ...
mock.Call3rdParty(ctx, argument).Return(nil).Toggle(Env())

P.S. If there is an easier way to achieve similar behaviour than what I proposed, please let me know. I wasn't able to find any workaround, other than calling Unset in if block. I am also open to any suggestions and would definitely like to implement this logic somewhere in testify package, only if you think this is the right approach.

Related issues

None

@piotrpersona piotrpersona force-pushed the feature/configure-unset-option branch 2 times, most recently from deb7fb9 to 6f8b398 Compare May 30, 2024 12:47
Consider the following example test scenario:
The goal is to test http server API. The test may set up a local http
server, database and mocks to 3rd party services. Since the tests are
calling mocked server it may be useful to use mock `EXPECT()` calls.
Test structure:
1. Setup local server, database and mocks
2. Mock 3rd party service calls
3. Send request to local server
4. Assert response
5. Verify side effects - execute database query

As an enhancement, such tests can easily be called against deployed
application, by simply replacing server and database URLs.
The only catch are mocked calls, which won't be executed, since the
request is sent to a remote server.

In such case it may be possible to call `Unset()` method with
`WithUnsetToggle` option, which value is set based on the current
environment. For example:
```go
func Env() mock.UnsetOption {
    if os.Getenv("local") {
        return mock.WithUnsetToggle(false)
    }
    return mock.WithUnsetToggle(true)
}

// In test ...
func TestAPI(t *testing.T) {
    // setup server, database and mocks
    mock.Call3rdParty(ctx, argument).Return(nil).Unset(Env())

    response := client.Post(request)
    // assertions and database query
}
```

In the above example, if environment is "local" Unset will be disabled,
meaning that all mocks will execute.
If environment is other than "local" the Unset will be enabled, meaning
that mocks will not be called.

Signed-off-by: Piotr Persona <persona.piotr@gmail.com>
@piotrpersona piotrpersona force-pushed the feature/configure-unset-option branch from 6f8b398 to 035a12b Compare May 30, 2024 12:48
@piotrpersona piotrpersona marked this pull request as ready for review May 30, 2024 12:55
@piotrpersona piotrpersona changed the title feat: Conditionally enable/disable Unset call mock: Conditionally enable/disable Unset call May 30, 2024
Signed-off-by: Piotr Persona <persona.piotr@gmail.com>
@piotrpersona piotrpersona changed the title mock: Conditionally enable/disable Unset call mock: Conditionally enable/disable toggle mock Call May 30, 2024
@piotrpersona piotrpersona changed the title mock: Conditionally enable/disable toggle mock Call mock: Conditionally toggle mock Call May 30, 2024
@brackendawson
Copy link
Collaborator

calling Unset in if block

Please, just do that, it is far more readable. Better yet, inverse the logic of the if block and never set the expectation in the first place:

// less good
m := mock.Call3rdParty(ctx, argument).Return(nil)
if os.Getenv("local") {
  m.Unset()
}

// better
if !os.Getenv("local") {
  mock.Call3rdParty(ctx, argument).Return(nil).Once()
}

It is my opinion that Unset should not have been added to begin with, its use makes it difficult to grok what expectations a test actually has. I would urge us to not complicate it further, sorry.

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 this pull request may close these issues.

2 participants