better tests using new infra, and use DI rather than patching #5250

Merged
merged 1 commit into from May 10, 2016

Conversation

Projects
None yet
2 participants
Contributor

natefinch commented Apr 21, 2016

Small change to the Main function so that it returns its exit code, rather than calling os.Exit, since the latter makes calling it in tests impossible. Also change Main to call a sub-function main, which takes dependencies for mocking out exec.LookPath and exec.Command.

Fix the tests so that they use the functionality added to github.com/juju/testing in juju/testing#95 - use the new PatchExecHelper type and CaptureOutput function so that we can isolate this code without having to have all that complicated code right here in the tests (and thus it can be reused for other tests).

(Review request: http://reviews.vapour.ws/r/4677/)

Contributor

natefinch commented May 9, 2016

$$fixes-1579059$$

Contributor

jujubot commented May 9, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented May 9, 2016

Build failed: Generating tarball failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/7688

Contributor

natefinch commented May 9, 2016

$$merge$$

Contributor

jujubot commented May 9, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented May 9, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/7690

Contributor

natefinch commented May 9, 2016

$$merge$$

Contributor

jujubot commented May 9, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented May 9, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/7691

Contributor

natefinch commented May 10, 2016

$$nobutseriously$$

Contributor

jujubot commented May 10, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 56de25f into juju:master May 10, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment