add PatchPath method

add PatchPath method

This consolidates a bunch of spots in tests that all just watch to patch the $PATH (and were doing so in a non-portable way).

https://code.launchpad.net/~natefinch/juju-core/035-pathlists/+merge/206984

(do not edit description out of merge proposal)

[natefinch, 2014-02-18 16:32:24]

Please take a look.

[rog, 2014-02-18 16:41:40]

LGTM except that I'd suggest a slightly more meaningful name,
say PatchExecPath, because there are many "paths" around the
place, and it's perhaps not always obvious when Path refers
to the shell's $PATH.

One other suggestion below.

https://codereview.appspot.com/65460043/diff/1/environs/sshstorage/storage_te...
File environs/sshstorage/storage_test.go (right):

https://codereview.appspot.com/65460043/diff/1/environs/sshstorage/storage_te...
environs/sshstorage/storage_test.go:65: restoreEnv := testbase.PatchPath(s.bin)
couldn't this just use s.PatchPath and lose the restoreEnv variable?

[dimitern, 2014-02-18 16:45:38]

LGTM with a few suggestions.

https://codereview.appspot.com/65460043/diff/1/testing/testbase/patch.go
File testing/testbase/patch.go (right):

https://codereview.appspot.com/65460043/diff/1/testing/testbase/patch.go#newc...
testing/testbase/patch.go:63: // PatchPath provides a test a simple way to
prepend path to the start of
s/a test//
s/prepend path/prepend a path/

https://codereview.appspot.com/65460043/diff/1/testing/testbase/patch.go#newc...
testing/testbase/patch.go:64: // the environment's $PATH variable. A function is
returned that will return the
s/environment's $PATH variable/PATH environment variable/ ?

s/A function is returned that will return/Returns a function that restores the/

https://codereview.appspot.com/65460043/diff/1/testing/testbase/patch.go#newc...
testing/testbase/patch.go:66: func PatchPath(dir string) Restorer {
s/PatchPath/PrependPath/ perhaps, because it more obviously describes what
happens?

https://codereview.appspot.com/65460043/diff/1/testing/testbase/patch_test.go
File testing/testbase/patch_test.go (right):

https://codereview.appspot.com/65460043/diff/1/testing/testbase/patch_test.go...
testing/testbase/patch_test.go:89: 
d

https://codereview.appspot.com/65460043/diff/1/testing/testbase/patch_test.go...
testing/testbase/patch_test.go:93: 
expectPath := dir+string(os.PathListSeparator)+oldPath
c.Check(os.Getenv("PATH"), gc.Equals, expectPath)
?

[natefinch, 2014-02-18 17:10:32]

Please take a look.