Make tests pass on raring

Make tests pass on raring

Raring ships with a later version of git (1.8.x).
This newer version requires user and email to be 
known before committing and on some raring setups,
the info could not be found. So I added a test
fixture to ensure the info was available when the
tests are run.

https://code.launchpad.net/~wallyworld/juju-core/fix-raring-tests/+merge/153488

(do not edit description out of merge proposal)

[wallyworld, 2013-03-15 01:12:44]

Please take a look.

[dave_cheney.net, 2013-03-15 01:25:02]

Awesome work, Thumper will be pleased. You might be able to make this diff more
compact with these minor suggestions.

https://codereview.appspot.com/7677044/diff/1/cmd/jujud/unit_test.go
File cmd/jujud/unit_test.go (right):

https://codereview.appspot.com/7677044/diff/1/cmd/jujud/unit_test.go#newcode13
cmd/jujud/unit_test.go:13: testing.GitSuite
if you rename this to

GitSuite testing.GitSuite

https://codereview.appspot.com/7677044/diff/1/cmd/jujud/unit_test.go#newcode20
cmd/jujud/unit_test.go:20: s.agentSuite.SetUpSuite(c)
then you can remove this

https://codereview.appspot.com/7677044/diff/1/cmd/jujud/unit_test.go#newcode24
cmd/jujud/unit_test.go:24: s.agentSuite.TearDownSuite(c)
and this

https://codereview.appspot.com/7677044/diff/1/testing/git.go
File testing/git.go (right):

https://codereview.appspot.com/7677044/diff/1/testing/git.go#newcode19
testing/git.go:19: t.oldValues = make(map[string]string)
// or put them in a var at the top of the file

for _, v := range []string { "GIT_AUTHOR_NAME", "GIT_AUTHOR_EMAIL" ... } {
   t.oldValues[v] = os.Getenv(v)
}

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/deployer_te...
File worker/uniter/charm/deployer_test.go (right):

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/deployer_te...
worker/uniter/charm/deployer_test.go:20: s.GitSuite.SetUpSuite(c)
not sure these is needed

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/deployer_te...
worker/uniter/charm/deployer_test.go:24: s.GitSuite.TearDownSuite(c)
same

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/deployer_te...
worker/uniter/charm/deployer_test.go:28: s.GitSuite.SetUpTest(c)
same

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/deployer_te...
worker/uniter/charm/deployer_test.go:32: s.GitSuite.TearDownTest(c)
same

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/git_test.go
File worker/uniter/charm/git_test.go (right):

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/git_test.go...
worker/uniter/charm/git_test.go:17: testing.LoggingSuite
LoggingSuite testing.LoggingSuite will break the ambiguity

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/git_test.go...
worker/uniter/charm/git_test.go:24: s.GitSuite.SetUpSuite(c)
rm

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/git_test.go...
worker/uniter/charm/git_test.go:28: s.GitSuite.TearDownSuite(c)
rm

[wallyworld, 2013-03-15 02:06:32]

Thanks for the great tips. All fixed.

https://codereview.appspot.com/7677044/diff/1/cmd/jujud/unit_test.go
File cmd/jujud/unit_test.go (right):

https://codereview.appspot.com/7677044/diff/1/cmd/jujud/unit_test.go#newcode13
cmd/jujud/unit_test.go:13: testing.GitSuite
On 2013/03/15 01:25:02, dfc wrote:
> if you rename this to
> 
> GitSuite testing.GitSuite

Done.

https://codereview.appspot.com/7677044/diff/1/cmd/jujud/unit_test.go#newcode20
cmd/jujud/unit_test.go:20: s.agentSuite.SetUpSuite(c)
On 2013/03/15 01:25:02, dfc wrote:
> then you can remove this 

Done.

https://codereview.appspot.com/7677044/diff/1/cmd/jujud/unit_test.go#newcode24
cmd/jujud/unit_test.go:24: s.agentSuite.TearDownSuite(c)
On 2013/03/15 01:25:02, dfc wrote:
> and this

Done.

https://codereview.appspot.com/7677044/diff/1/testing/git.go
File testing/git.go (right):

https://codereview.appspot.com/7677044/diff/1/testing/git.go#newcode19
testing/git.go:19: t.oldValues = make(map[string]string)
On 2013/03/15 01:25:02, dfc wrote:
> // or put them in a var at the top of the file
> 
> for _, v := range []string { "GIT_AUTHOR_NAME", "GIT_AUTHOR_EMAIL" ... } {
>    t.oldValues[v] = os.Getenv(v)
> }

Done.

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/deployer_te...
File worker/uniter/charm/deployer_test.go (right):

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/deployer_te...
worker/uniter/charm/deployer_test.go:20: s.GitSuite.SetUpSuite(c)
On 2013/03/15 01:25:02, dfc wrote:
> not sure these is needed

Done.

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/deployer_te...
worker/uniter/charm/deployer_test.go:20: s.GitSuite.SetUpSuite(c)
On 2013/03/15 01:25:02, dfc wrote:
> not sure these is needed

Done.

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/deployer_te...
worker/uniter/charm/deployer_test.go:24: s.GitSuite.TearDownSuite(c)
On 2013/03/15 01:25:02, dfc wrote:
> same

Done.

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/deployer_te...
worker/uniter/charm/deployer_test.go:28: s.GitSuite.SetUpTest(c)
On 2013/03/15 01:25:02, dfc wrote:
> same

Done.

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/deployer_te...
worker/uniter/charm/deployer_test.go:32: s.GitSuite.TearDownTest(c)
On 2013/03/15 01:25:02, dfc wrote:
> same

Done.

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/git_test.go
File worker/uniter/charm/git_test.go (right):

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/git_test.go...
worker/uniter/charm/git_test.go:17: testing.LoggingSuite
On 2013/03/15 01:25:02, dfc wrote:
> LoggingSuite testing.LoggingSuite will break the ambiguity

Done.

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/git_test.go...
worker/uniter/charm/git_test.go:24: s.GitSuite.SetUpSuite(c)
On 2013/03/15 01:25:02, dfc wrote:
> rm

Done.

https://codereview.appspot.com/7677044/diff/1/worker/uniter/charm/git_test.go...
worker/uniter/charm/git_test.go:28: s.GitSuite.TearDownSuite(c)
On 2013/03/15 01:25:02, dfc wrote:
> rm

Done.

[wallyworld, 2013-03-15 02:06:48]

Please take a look.

[dave_cheney.net, 2013-03-15 02:16:28]

LGTM. You can simplify a little more if you fancy it.

https://codereview.appspot.com/7677044/diff/9001/cmd/jujud/unit_test.go
File cmd/jujud/unit_test.go (right):

https://codereview.appspot.com/7677044/diff/9001/cmd/jujud/unit_test.go#newco...
cmd/jujud/unit_test.go:13: GitSuite testing.GitSuite
see comment in next file

https://codereview.appspot.com/7677044/diff/9001/testing/git.go
File testing/git.go (right):

https://codereview.appspot.com/7677044/diff/9001/testing/git.go#newcode11
testing/git.go:11: 
// please comment about why these vars are listed as a hint when we need to add
more in the future.

https://codereview.appspot.com/7677044/diff/9001/testing/git.go#newcode13
testing/git.go:13: "GIT_AUTHOR_NAME", "GIT_AUTHOR_EMAIL", "GIT_COMMITTER_NAME",
"GIT_COMMITTER_EMAIL",
nit: put each on a new line for reability

https://codereview.appspot.com/7677044/diff/9001/testing/git.go#newcode16
testing/git.go:16: func (t *GitSuite) SetUpSuite(c *C) {}
drop, then you can drop the renaming in jujud/unit_test.go

https://codereview.appspot.com/7677044/diff/9001/testing/git.go#newcode18
testing/git.go:18: func (t *GitSuite) TearDownSuite(c *C) {}
drop

[thumper, 2013-03-15 02:42:42]

LGTM - thanks Ian.

[wallyworld, 2013-03-15 02:58:37]

*** Submitted:

Make tests pass on raring

Raring ships with a later version of git (1.8.x).
This newer version requires user and email to be 
known before committing and on some raring setups,
the info could not be found. So I added a test
fixture to ensure the info was available when the
tests are run.

R=dfc, thumper
CC=
https://codereview.appspot.com/7677044

https://codereview.appspot.com/7677044/diff/9001/cmd/jujud/unit_test.go
File cmd/jujud/unit_test.go (right):

https://codereview.appspot.com/7677044/diff/9001/cmd/jujud/unit_test.go#newco...
cmd/jujud/unit_test.go:13: GitSuite testing.GitSuite
On 2013/03/15 02:16:29, dfc wrote:
> see comment in next file

Done.

https://codereview.appspot.com/7677044/diff/9001/testing/git.go
File testing/git.go (right):

https://codereview.appspot.com/7677044/diff/9001/testing/git.go#newcode11
testing/git.go:11: 
On 2013/03/15 02:16:29, dfc wrote:
> // please comment about why these vars are listed as a hint when we need to
add
> more in the future.

Done.

https://codereview.appspot.com/7677044/diff/9001/testing/git.go#newcode13
testing/git.go:13: "GIT_AUTHOR_NAME", "GIT_AUTHOR_EMAIL", "GIT_COMMITTER_NAME",
"GIT_COMMITTER_EMAIL",
On 2013/03/15 02:16:29, dfc wrote:
> nit: put each on a new line for reability

Done.

https://codereview.appspot.com/7677044/diff/9001/testing/git.go#newcode16
testing/git.go:16: func (t *GitSuite) SetUpSuite(c *C) {}
On 2013/03/15 02:16:29, dfc wrote:
> drop, then you can drop the renaming in jujud/unit_test.go

Done.

https://codereview.appspot.com/7677044/diff/9001/testing/git.go#newcode18
testing/git.go:18: func (t *GitSuite) TearDownSuite(c *C) {}
On 2013/03/15 02:16:29, dfc wrote:
> drop

Done.

[jameinel, 2013-03-17 11:17:55]

I really like that the tests are passing, I'm wondering if we will run into
production issues if someone tries to deploy to a Raring target. Since a newly
created target instance will probably not have 'git' env vars set up.

I don't know that for sure, and certainly this is a reasonable first step. We
can always come back to address Raring when we need to.