29

suppose that we have a method like this:

func method(intr MyInterface) {
    go intr.exec()
} 

In unit testing method, we want to assert that inter.exec has been called once and only once; so we can mock it with another mock struct in tests, which will give us functionality to check if it has been called or not:

type mockInterface struct{
    CallCount int
}

func (m *mockInterface) exec() {
    m.CallCount += 1
}

And in unit tests:

func TestMethod(t *testing.T) {
    var mock mockInterface{}
    method(mock)
    if mock.CallCount != 1 {
        t.Errorf("Expected exec to be called only once but it ran %d times", mock.CallCount)
    }
}

Now, the problem is that since intr.exec is being called with go keyword, we can't be sure that when we reach our assertion in tests, it has been called or not.

Possible Solution 1:

Adding a channel to arguments of intr.exec may solve this: we could wait on receiving any object from it in tests, and after receiving an object from it then we could continue to assert it being called. This channel will be completely unused in production (non-test) codes. This will work but it adds unnecessary complexity to non-test codes, and may make large codebases incomprehensible.

Possible Solution 2:

Adding a relatively small sleep to tests before assertion may give us some assurance that the goroutine will be called before sleep is finished:

func TestMethod(t *testing.T) {
    var mock mockInterface{}
    method(mock)

    time.sleep(100 * time.Millisecond)

    if mock.CallCount != 1 {
        t.Errorf("Expected exec to be called only once but it ran %d times", mock.CallCount)
    }
}

This will leave non-test codes as they are now.
The problem is that it will make tests slower, and will make them flaky, since they may break in some random circumstances.

Possible Solution 3:

Creating a utility function like this:

var Go = func(function func()) {
    go function()
} 

And rewrite method like this:

func method(intr MyInterface) {
    Go(intr.exec())
} 

In tests, we could change Go to this:

var Go = func(function func()) {
    function()
} 

So, when we're running tests, intr.exec will be called synchronously, and we can be sure that our mock method is called before assertion.
The only problem of this solution is that it's overriding a fundamental structure of golang, which is not right thing to do.


These are solutions that I could find, but non are satisfactory as far as I can see. What is best solution?

sazary
  • 906
  • 1
  • 9
  • 20
  • A small sleep is generally sufficient and shouldn't have a significant impact on test execution times. There's also [`runtime.Gosched()`](https://golang.org/pkg/runtime/#Gosched) which yields to another goroutine without any sleep delay at all. – Adrian Jun 27 '18 at 14:36
  • 1
    IMO using a sleep for synchronization in tests is a code smell that will often fail in the future. Create some sort of synchronized side effect that you can observe in the tests. – JimB Jun 27 '18 at 14:42
  • @JimB my code (my business logic) doesn't require any synchronized side effect. I much prefer not to add them merely due to some testing problem. – sazary Jun 27 '18 at 15:05
  • @Adrian that's a nice idea, but `Gosched` won't allow me to select which goroutine is selected, am I right? We're running our tests in parallel, and I fear that creates problems – sazary Jun 27 '18 at 15:06
  • 1
    Neither does sleep. No matter what you use, you cannot control the behavior of the scheduler. – Adrian Jun 27 '18 at 15:07
  • @sazary: if there are no observable side effects, then the code is not reliably testable. If you want to write testable code, there are some concessions you have to make. Having a small test hook to ensure something happened is not asking much, and is a common pattern. – JimB Jun 27 '18 at 15:09
  • @JimB I don't think adding a side effect is best solution. Usually we change the target method with a "spy" or mock, and part of it's job is to tell us how many times it has been called and with which arguments it has been called. – sazary Jun 28 '18 at 07:41

3 Answers3

25

Use a sync.WaitGroup inside the mock

You can extend mockInterface to allow it to wait for the other goroutine to finish

type mockInterface struct{
    wg sync.WaitGroup // create a wait group, this will allow you to block later
    CallCount int
}

func (m *mockInterface) exec() {
    m.wg.Done() // record the fact that you've got a call to exec
    m.CallCount += 1
}

func (m *mockInterface) currentCount() int {
    m.wg.Wait() // wait for all the call to happen. This will block until wg.Done() is called.
    return m.CallCount
}

In the tests you can do:

mock := &mockInterface{}
mock.wg.Add(1) // set up the fact that you want it to block until Done is called once.

method(mock)

if mock.currentCount() != 1 {  // this line with block
    // trimmed
}
Zak
  • 5,515
  • 21
  • 33
  • thank you for your comment, but as far as I can see, it's not different that much from 1st solution that I have written, and it has they same draw backs, doesn't it? – sazary Jun 27 '18 at 15:04
  • 2
    it's similar in the sense that you are using synchronisation techniques. Maybe something is unclear in my answer, but the method described above encapsulates all the synchronisation inside the mock, and you do not pollute the production code. – Zak Jun 27 '18 at 15:14
  • this solution has few problems: 1) test will hang forever if goroutine is not called, 2) test will panic if the goroutine has been called more than once – Maxim Jun 28 '18 at 05:23
  • 3) m.CallCount += 1 is not safe because there can be a race if exec method is called in multiple goroutines – Maxim Jun 28 '18 at 05:31
  • @Zak you're right, this is better than my 1st solution – sazary Jun 28 '18 at 07:48
  • @Maxim your points are valid, and I think Zak has removed them for brevity – sazary Jun 28 '18 at 07:49
  • @sazary I don't think Zak has removed anything because there's simply no solution for 1 and 2 with sync.WaitGroup. – Maxim Jun 28 '18 at 14:04
4

This test won't hang forever as with sync.WaitGroup solution proposed above. It will hang for a second (in this particular example) in case when there is no call to mock.exec:

package main

import (
    "testing"
    "time"
)

type mockInterface struct {
    closeCh chan struct{}
}

func (m *mockInterface) exec() {
    close(closeCh)
}

func TestMethod(t *testing.T) {
    mock := mockInterface{
        closeCh: make(chan struct{}),
    }

    method(mock)

    select {
    case <-closeCh:
    case <-time.After(time.Second):
        t.Fatalf("expected call to mock.exec method")
    }
}

This is basically what mc.Wait(time.Second) in my answer above.

Maxim
  • 514
  • 2
  • 7
  • adding timeouts will not enforce the goroutines to be scheduled, and therefore the calls to `exce()` to happen. It only *increases the chances* of it happening. Timeouts are always the wrong solution, because optimising the timeout for different platforms, CPUs, execution times etc is impossible – Zak Jun 28 '18 at 08:48
  • In this case timeout doesn't matter because here we have reads from the channels which enforce goroutines to be scheduled. – Maxim Jun 28 '18 at 12:00
  • Sure, but nothing forces the call to `exec` to happen in less than one second. You just hope that I _probably_ will happen. Logically there's no way to assume that it will. – Zak Jun 28 '18 at 12:56
  • Yes, and you hope the test will succeed in your example above. Logically there's no way to assume that wg.Wait will return, right? Reasonable timeout here is just a protection from the test that can hang forever. In my case it will hang for a second, fail and go test will continue to execute remaining tests. – Maxim Jun 28 '18 at 13:12
  • 1
    In the answer I gave, and the scenario described by you here; In my case, it's a programming error. But the behaviour is well defined and deterministic. In your case it's a scheduler that's to blame and the behaviour is undefined and non-deterministic. Take your pick! – Zak Jun 28 '18 at 13:22
  • 1
    Practically that what will happen with you answer: you will stuck waiting for the failed test forever. Yes it's a programming error but you'll have to find it yourself because there won't be clear output from go test. In my case there is practical 1 second deadline that will work only for FAILED test, after test hit this deadline there will be clear output from go test command. In practice I have never experienced any problems with such approach because 1 second is more than enough. But yes, theoretically it's not deterministic. So keep theoretisizing. – Maxim Jun 28 '18 at 13:59
  • If we are testing code which will actually be used in a real live scenario, every method will implicitly have some kind of deadline, after which every human tester would consider the method failed. If I write unit tests for e.g. a webserver and the whole API-call will have a timeout of 3 seconds, it is probably safe to fail the test of a single subroutine if it is not executed in 1s on the testing machine. I want this test to fails we will have to change the underlying code to fullfil our business requirements. – Falco Mar 13 '20 at 14:45
  • And if the go-runtime in use can not provide the functionality to schedule a simple routine on the well defined testing-machine with enough resources in under 1s 99.999% of the time, it is not suited for most real life requirements (even if the specs would theoretically still call this runtime compliant) – Falco Mar 13 '20 at 14:47
-1

first of all I would use a mock generator, i.e. github.com/gojuno/minimock instead of writing mocks yourself:

minimock -f example.go -i MyInterface -o my_interface_mock_test.go

then your test can look like this (btw the test stub is also generated with github.com/hexdigest/gounit)

func Test_method(t *testing.T) {
    type args struct {
        intr MyInterface
    }
    tests := []struct {
        name string
        args func(t minimock.Tester) args
    }{
        {
            name: "check if exec is called",
            args: func(t minimock.Tester) args {
                return args{
                    intr: NewMyInterfaceMock(t).execMock.Return(),
                }
            },
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            mc := minimock.NewController(t)
            defer mc.Wait(time.Second)

            tArgs := tt.args(mc)

            method(tArgs.intr)
        })
    }
}

In this test

defer mc.Wait(time.Second)

Waits for all mocked methods to be called.

Maxim
  • 514
  • 2
  • 7