Code review comment for lp:~themue/golxc/003-added-network

William Reade (fwereade) wrote :

I really think we should have tests that run without sudo -- so, just
verifying what commands we call -- that we can run for sanity's sake
before exercising the package against our own machines.

And, for the tests that do hit the real system, I feel we should drop
golxc_test.sh and use something that lets us run "go test ./..." for the
unit tests and `sudo go test ./... -sudo` (or similar) to run the real
tests as well.

A few comments below

https://codereview.appspot.com/6849102/diff/5002/golxc.go
File golxc.go (right):

https://codereview.appspot.com/6849102/diff/5002/golxc.go#newcode311
golxc.go:311: out, err := cmd.CombinedOutput()
Again, I'm not sure about CombinedOutput. If it's sane, there should be
a comment explaining why the streams cannot pollute one another in
confusing ways.

https://codereview.appspot.com/6849102/diff/5002/golxc_test.go
File golxc_test.go (right):

https://codereview.appspot.com/6849102/diff/5002/golxc_test.go#newcode140
golxc_test.go:140: f, err := ioutil.TempFile("/tmp", "lxc-test")
c.MkDir()

https://codereview.appspot.com/6849102/diff/5002/golxc_test.go#newcode142
golxc_test.go:142: defer func() {
...then you can drop this defer, the test will clean up for you.

https://codereview.appspot.com/6849102/diff/5002/golxc_test.go#newcode160
golxc_test.go:160: orig :=
golxc.SetLXCDefaultFile("/tmp/not-existing-exc-test")
c.MkDir()

https://codereview.appspot.com/6849102/

« Back to merge proposal