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 issues in Go 1.5 #553

Closed
aaronkavlie-wf opened this issue Oct 16, 2015 · 6 comments
Closed

Mock issues in Go 1.5 #553

aaronkavlie-wf opened this issue Oct 16, 2015 · 6 comments

Comments

@aaronkavlie-wf
Copy link
Contributor

Several of our tests using github.com/Shopify/sarama/mocks are failing in Go 1.5.1 due to count mismatches between expected/actual offsets and the like. The same test suite passes when run with Go 1.4.2.

Have you seen any difference in test code that uses mocks with Go 1.5?

@eapache
Copy link
Contributor

eapache commented Oct 16, 2015

@wvanbergen ?

Off the top of my head it sounds like a race condition exposed by 1.5's change to the default GOMAXPROCS. Does adding -race to the test expose anything? Is this producer or consumer?

@aaronkavlie-wf
Copy link
Contributor Author

I'm seeing the test failures in the file that covers our sarama extraction -- both the producer and consumer are used there.

A race condition is likely. I commented out all but the first test, and saw a panic due to nil pointer at first. After adding a logging statement to the sarama source, then removing it, the test now fails with no panic. And running with -race made the test pass. Very odd.

I have also tried running with env GOMAXPROCS=1 which hasn't made a difference.

@aaronkavlie-wf
Copy link
Contributor Author

I just saw it fail, and then pass, with the -race flag and no changes.

Further evidence of a race condition:

    <autogenerated>:31: Unexpected offset when calling ConsumePartition for test/0. Expected 4, got 1.
    <autogenerated>:31: Unexpected offset when calling ConsumePartition for test/1. Expected 1, got 4.
    buffer_test.go:258: Next offset expected to be 4; actual: 1
    buffer_test.go:263: Next offset expected to be 1; actual: 4

@eapache
Copy link
Contributor

eapache commented Oct 19, 2015

It's not super-clear to me, but based on those logs could it be an actual bug in your code that's been uncovered? The mock code involved is fairly simple and protected by a mutex: you're telling it to expect test/0 with offset 4, and then actually calling ConsumePartition with offset 1... and vice-versa for test/1.

@aaronkavlie-wf
Copy link
Contributor Author

@eapache I have found a race condition in my code; I was naively assuming loop index order was preserved.

I have another race condition which is not so simple however. Tests now pass most of the time, but sometimes, some of the messages inserted with mocks.PartitionConsumer.YieldMessage() are not ready when I check partitionConsumer.Messages(). Is there any way to ensure that the test messages have been added to the channel?

@eapache
Copy link
Contributor

eapache commented Oct 27, 2015

I don't see one off the top of my head. @wvanbergen why does YieldMessage go through handleExpectations instead of just writing straight to messages?

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

No branches or pull requests

2 participants