{"action":"created","issue":{"url":"https://api.github.com/repos/juju/juju/issues/1619","labels_url":"https://api.github.com/repos/juju/juju/issues/1619/labels{/name}","comments_url":"https://api.github.com/repos/juju/juju/issues/1619/comments","events_url":"https://api.github.com/repos/juju/juju/issues/1619/events","html_url":"https://github.com/juju/juju/pull/1619","id":58050446,"number":1619,"title":"Small cleanups to EnvWorkerManager","user":{"login":"jameinel","id":202877,"avatar_url":"https://avatars.githubusercontent.com/u/202877?v=3","gravatar_id":"","url":"https://api.github.com/users/jameinel","html_url":"https://github.com/jameinel","followers_url":"https://api.github.com/users/jameinel/followers","following_url":"https://api.github.com/users/jameinel/following{/other_user}","gists_url":"https://api.github.com/users/jameinel/gists{/gist_id}","starred_url":"https://api.github.com/users/jameinel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jameinel/subscriptions","organizations_url":"https://api.github.com/users/jameinel/orgs","repos_url":"https://api.github.com/users/jameinel/repos","events_url":"https://api.github.com/users/jameinel/events{/privacy}","received_events_url":"https://api.github.com/users/jameinel/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-02-18T10:27:56Z","updated_at":"2015-02-19T10:57:01Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/juju/juju/pulls/1619","html_url":"https://github.com/juju/juju/pull/1619","diff_url":"https://github.com/juju/juju/pull/1619.diff","patch_url":"https://github.com/juju/juju/pull/1619.patch"},"body":"This is a small bit of changes to improve cleanups wrt EnvWorkerManager and its tests.\r\n\r\n1) Instead of calling m.tomb.Done() after m.tomb.Kill() use a defer m.tomb.Done(). I audited the code and the other places use the defer syntax. This lets a panic() enforce that the tomb will be finished.\r\n\r\n2) change waitOrPanic into waitOrFatal(c, ...) which lets us just call c.Fatal rather than triggering a panic inside the test suite.\r\n\r\n3) Stop requiring a Dying() as part of the Runner interface just to allow EnvWorkerManager to shutdown if its runner is stopping. Other code that does something similary uses:\r\n  go func() { my.tomb.Kill(dependent.Wait()) }\r\nSo that if the thing I depend on stops, I will kill my own tomb.\r\nThis lets us shrink the interface, and it makes this code conform to the other changes.\r\n\r\n4) Add defer m.runner.Kill() as well, which ensures that even if our loop exits because of a different error, we make sure to shut down our runner.\r\n\r\nAssociated with this is a test which actually exercises that code path by injecting an error. It is is a bit more coupled to implementation than I'd like, but I don't think injecting errors to test error handling can be done a lot better than that.\r\n\n\n(Review request: http://reviews.vapour.ws/r/949/)"},"comment":{"url":"https://api.github.com/repos/juju/juju/issues/comments/75033590","html_url":"https://github.com/juju/juju/pull/1619#issuecomment-75033590","issue_url":"https://api.github.com/repos/juju/juju/issues/1619","id":75033590,"user":{"login":"jujubot","id":7779494,"avatar_url":"https://avatars.githubusercontent.com/u/7779494?v=3","gravatar_id":"","url":"https://api.github.com/users/jujubot","html_url":"https://github.com/jujubot","followers_url":"https://api.github.com/users/jujubot/followers","following_url":"https://api.github.com/users/jujubot/following{/other_user}","gists_url":"https://api.github.com/users/jujubot/gists{/gist_id}","starred_url":"https://api.github.com/users/jujubot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jujubot/subscriptions","organizations_url":"https://api.github.com/users/jujubot/orgs","repos_url":"https://api.github.com/users/jujubot/repos","events_url":"https://api.github.com/users/jujubot/events{/privacy}","received_events_url":"https://api.github.com/users/jujubot/received_events","type":"User","site_admin":false},"created_at":"2015-02-19T10:57:01Z","updated_at":"2015-02-19T10:57:01Z","body":"Build failed: Tests failed\n        build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/2188\n"}}