Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Fix test race, and remove JujuConnSuite dependency. #7742
Conversation
| s.waitLoggingInfo(c, expected) | ||
| + worker.Stop(loggingWorker) |
howbazaar
Aug 15, 2017
Owner
That was the race. The worker may update the value in another goroutine. Stop the worker before confirming the expected callback.
| + return m.config, nil | ||
| +} | ||
| +func (m *mockAPI) WatchLoggingConfig(agentTag names.Tag) (watcher.NotifyWatcher, error) { | ||
| + return m.watcher, nil |
wallyworld
Aug 15, 2017
Owner
we should set up the mock api with the expected agent and then assert here that the agent matches to make sure we are passing in the expected value, or just assert that it equals 42 as that's what was set up earlier
howbazaar
Aug 15, 2017
Owner
We can record the agent tag passed in and confirm in the test, but the gc.C that we have in the setup doesn't work properly within the normal tests.
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
jujubot
merged commit 6a2a4b3
into
juju:2.2
Aug 15, 2017
1 check passed
continuous-integration/jenkins/pr-merge
This commit looks good
Details
howbazaar
deleted the
howbazaar:2.2-logger-worker-race
branch
Aug 15, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
howbazaar commentedAug 15, 2017
This branch fixes a test race that was introduced in the recent PR #7709.
As a drive by I removed the JujuConnSuite and introduced an interface for the API calls.