search

ravendb / ravendb-go-client

MIT License
39 stars 16 forks source link

TestSubscriptionsBasic flaky #149

Open kjk opened 5 years ago

kjk commented 5 years ago

https://ci.appveyor.com/project/ravendb/ravendb-go-client/builds/23144627/job/ge467894jiqskbqv?fullLog=true#L4358

Could be just networking issue with the machine.

--- FAIL: TestSubscriptionsBasic (5.84s)
    subscriptions_basic_test.go:286: 
            Error Trace:    subscriptions_basic_test.go:286
                                        asm_amd64.s:1333
            Error:          Not equal: 
                            expected: "Adam"
                            actual  : "James"

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -1 +1 @@
                            -Adam
                            +James
            Test:           TestSubscriptionsBasic
    subscriptions_basic_test.go:286: 
            Error Trace:    subscriptions_basic_test.go:286
                                        asm_amd64.s:1333
            Error:          Not equal: 
                            expected: "David"
                            actual  : "Adam"

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -1 +1 @@
                            -David
                            +Adam
            Test:           TestSubscriptionsBasic
=== RUN   TestSuggestionsLazy
ayende commented 5 years ago

Try logging all the previous values that you go there. It might have called this twice? Or there was an error and it retired?

kjk commented 5 years ago

It looks like (sometimes) we don't get the first expected value, created before subscription.Run.

ayende commented 5 years ago

In here: https://github.com/ravendb/ravendb-go-client/blob/master/tests/subscriptions_basic_test.go#L271

And: https://github.com/ravendb/ravendb-go-client/blob/master/subscription_worker.go#L427

You are putting the batch into a channel, but it looks like this instance is being reused, so you are getting data races. If this is a common setup in Go, this is probably a real pitfall and we need to make sure that we won't be re-using this instance.

kjk commented 5 years ago

For that reason the callback receives a copy of SubscriptionBatch: https://github.com/ravendb/ravendb-go-client/blob/master/subscription_worker.go#L444

ayende commented 5 years ago

But you are passing the Item instance, isn't that mutable?

kjk commented 5 years ago

.Items gets re-created from scratch in each iteration in https://sourcegraph.com/github.com/ravendb/ravendb-go-client@master/-/blob/subscription_batch.go#L87 called from https://sourcegraph.com/github.com/ravendb/ravendb-go-client@2c2cde5258d42e531d03f95fa98713cb6e1b8135/-/blob/subscription_worker.go#L437

ayende commented 5 years ago

Okay, make sense.

When you check the logs, see if you can also log what comes from the network. I think this is a Go client problem, not a server one