Make FakeHome respect JUJU_HOME.

Make FakeHome respect JUJU_HOME.

Initial work was to fix bug 1171671.  However I then realised that I was
wrong. However I have cleaned up the FakeHome code to move JUJU_HOME out of
the say for the duration of the FakeHome existing.  I also finally got around
to unifying the fakeHomes.  The last remaining fakeHome was in config tests,
and it needed to handle files, so the testing.FakeHome now has that facility.

https://code.launchpad.net/~thumper/juju-core/fix-fake-home-with-juju-home-set/+merge/160256

(do not edit description out of merge proposal)

[thumper, 2013-04-23 03:48:01]

Please take a look.

[dimitern, 2013-04-23 11:07:29]

LGTM, thanks for the improvement, just a small suggestion.

https://codereview.appspot.com/8551047/diff/1/testing/environ.go
File testing/environ.go (right):

https://codereview.appspot.com/8551047/diff/1/testing/environ.go#newcode125
testing/environ.go:125: err = ioutil.WriteFile(path, []byte(f.Data), 0666)
for ssh keys we need to set chmod 0600, so why not have Mode as part of the
TestFile and use it here?

[TheMue, 2013-04-23 11:40:27]

LGTM with some small notes.

https://codereview.appspot.com/8551047/diff/1/testing/environ.go
File testing/environ.go (right):

https://codereview.appspot.com/8551047/diff/1/testing/environ.go#newcode52
testing/environ.go:52: files          []TestFile
Maybe a map[string]TestFile is better for checking if a file exists (only when
the number of files is large).

https://codereview.appspot.com/8551047/diff/1/testing/environ.go#newcode101
testing/environ.go:101: oldJujuHomeEnv := os.Getenv("JUJU_HOME")
Good catch.

https://codereview.appspot.com/8551047/diff/1/testing/environ.go#newcode125
testing/environ.go:125: err = ioutil.WriteFile(path, []byte(f.Data), 0666)
On 2013/04/23 11:07:29, dimitern wrote:
> for ssh keys we need to set chmod 0600, so why not have Mode as part of the
> TestFile and use it here?

Sounds useful, when the according tests check for the file mode too.

[thumper, 2013-04-26 01:34:26]

https://codereview.appspot.com/8551047/diff/1/testing/environ.go
File testing/environ.go (right):

https://codereview.appspot.com/8551047/diff/1/testing/environ.go#newcode52
testing/environ.go:52: files          []TestFile
On 2013/04/23 11:40:27, TheMue wrote:
> Maybe a map[string]TestFile is better for checking if a file exists (only when
> the number of files is large).

For the number of values we are iterating through, I don't expect this to ever
be an issue.  If we find ourselves writing enough that it will matter, we should
probably look at the test.

https://codereview.appspot.com/8551047/diff/1/testing/environ.go#newcode125
testing/environ.go:125: err = ioutil.WriteFile(path, []byte(f.Data), 0666)
On 2013/04/23 11:40:27, TheMue wrote:
> On 2013/04/23 11:07:29, dimitern wrote:
> > for ssh keys we need to set chmod 0600, so why not have Mode as part of the
> > TestFile and use it here?
> 
> Sounds useful, when the according tests check for the file mode too.

The behaviour is exactly as it was before.  We don't ever write files that we
need to check the mode for.  Also, since we don't have default args, we can't
have it specified for only some TestFiles.

For tests that need to care about the mode, I'd suggest extending the FakeHome,
or simply have the writing done elsewhere.

[thumper, 2013-04-26 01:42:33]

*** Submitted:

Make FakeHome respect JUJU_HOME.

Initial work was to fix bug 1171671.  However I then realised that I was
wrong. However I have cleaned up the FakeHome code to move JUJU_HOME out of
the say for the duration of the FakeHome existing.  I also finally got around
to unifying the fakeHomes.  The last remaining fakeHome was in config tests,
and it needed to handle files, so the testing.FakeHome now has that facility.

R=dimitern, TheMue
CC=
https://codereview.appspot.com/8551047