matryer / vice

Go channels at horizontal scale (powered by message queues)
https://medium.com/@matryer/introducing-vice-go-channels-across-many-machines-bcac1147d7e2
Apache License 2.0
1.55k stars 79 forks source link

flaky tests because the stopped transport is reused in vicetest/test.go #8

Closed HeavyHorst closed 7 years ago

HeavyHorst commented 7 years ago

This function

func Transport(t *testing.T, transport vice.Transport) {
    testStandardTransportBehaviour(t, transport)
    testSendChannelsDontBlock(t, transport)
}

uses the same transport for both tests.The first test stops the transport at the end, which is followed by some closed channels.

I get some flaky tests in the nats client code:

    go func() {
        for {
            select {
            case <-t.stopPubChan:
                return
            case msg := <-ch:
                if err := c.Publish(name, msg); err != nil {
                    t.errChan <- vice.Err{Message: msg, Name: name, Err: err}
                }
            default:
            }
        }
    }()
=== RUN   TestTransport
        test.go:38: send channels shouldn't block

That is because the t.stopPubChan is already closed.

Maybe we should export these 2 functions

    testStandardTransportBehaviour(t, transport)
    testSendChannelsDontBlock(t, transport)

so that every queue can run these tests with a new transport.

matryer commented 7 years ago

Perhaps we could change the transport test to:

func Transport(t *testing.T, func() vice.Transport)

That way, the function could create the transport and we could call it twice from within there?

I prefer this because, in theory, we might add more test functions later.

HeavyHorst commented 7 years ago

That should work. I can put a pull request together if you like.

matryer commented 7 years ago

That would be great.

Also, we should probably use t.Run for each function.

matryer commented 7 years ago

Did this fix it?

HeavyHorst commented 7 years ago

With #9 i can't reproduce this anymore.