golang / mock

GoMock is a mocking framework for the Go programming language.
Apache License 2.0
9.26k stars 608 forks source link

Pass `t *testing.T` 'at the end' to allow mocks configuration before subtests #625

Closed qdm12 closed 2 years ago

qdm12 commented 2 years ago

Requested feature Allow the user to configure mocks before the actual (sub)test and hence without passing *testing.T to a controller. The *testing.T could be passed within the subtest instead, perhaps at the start of it.

Why the feature is needed For now the only way to manage mocks with subtests is to have the controller defined within the subtest. Some developers prefer to configure all the 'state' before launching the subtests in a for loop iterating over test cases. The problem is having the controller defined in the parent test:

Production code:

//go:generate mockgen -destination=somethinger_test.go -package $GOPACKAGE . Somethinger

type Somethinger interface {
    Do(n int)
}

Test code:

func Test_Mocks(t *testing.T) {
    ctrl := gomock.NewController(t)
    mockedBuffer := NewMockSomethinger(ctrl)
    mockedBuffer.EXPECT().Do(3)

    testCases := []int{0, 1, 2, 3, 4}

    for _, testCase := range testCases {
        testCase := testCase
        t.Run(fmt.Sprint(testCase), func(t *testing.T) {
            mockedBuffer.Do(testCase)
        })
    }
}

Will result in

=== RUN   Test_Mocks
=== RUN   Test_Mocks/0
=== CONT  Test_Mocks
    /workspace/my_test.go:27: Unexpected call to *trie.MockSomethinger.Do([0]) at /workspace/my_test.go:27 because:
        expected call at /workspace/my_test.go:20 doesn't match the argument at index 0.
        Got: 0 (int)
        Want: is equal to 3 (int)
=== CONT  Test_Mocks/0
    /workspace/testing.go:1169: test executed panic(nil) or runtime.Goexit: subtest may have called FailNow on a parent test
=== CONT  Test_Mocks
    /workspace/controller.go:137: missing call(s) to *trie.MockSomethinger.Do(is equal to 3 (int)) /workspace/my_test.go:20
    /workspace/controller.go:137: aborting test due to missing call(s)
--- FAIL: Test_Mocks (0.00s)
    --- FAIL: Test_Mocks/0 (0.00s)

And further test cases (1,2,3,4) will not run because the parent test got failed within a subtest.

(Optional) Proposed solution A clear description of a proposed method for adding this feature to gomock.

I would propose the following, but I'm happy to discuss other options, and maybe it's not feasible at all.

  1. Have the option to pass in a nil controller to NewMockController
  2. Add a method RegisterT(t *testing.T) on the controller to register a t *testing.T. This would be called in subtests for example.
  3. Add a check for a set *testing.T field, whenever the controller Call(...) or RecordCall(...) method is called. If it's not set, panic.

One could also replace this nil *testing.T by a custom type implemeting gomock.TestReporter such as *gomock.DelayedT for example.

Regarding parallel tests, it should be ensured a particular mock is not called in more than one subtest when running them in parallel. This is the part that might be a bit tricky to handle and may make the whole thing not viable.

With these changes, you could then have:

func Test_Mocks(t *testing.T) {
    ctrl := gomock.NewController(nil)
    mockedBuffer := NewMockSomethinger(ctrl)
    mockedBuffer.EXPECT().Do(3)

    testCases := []int{0, 1, 2, 3, 4}

    for _, testCase := range testCases {
        testCase := testCase
        t.Run(fmt.Sprint(testCase), func(t *testing.T) {
            ctrl.RegisterT(t)
            mockedBuffer.Do(testCase)
        })
    }
}
codyoss commented 2 years ago

Because of how validation and stuff works on the mock today I think it is best if they mocks are made per-subtest. They are not really meant to be reused today. I think that would be a feature in itself. The parallel bit would make things tricky. There are assumptions made with mock internals that would make this challenging.

qdm12 commented 2 years ago

Makes sense :+1: Actually using mock builder fields in test cases does the trick in some way, where you configure the mock in a function taking a controller as argument (at least). I wrote some gist about it here if that can be of any interest to anyone. Anyway, thanks for the feedback!