juju-core

Merge lp:~rogpeppe/juju-core/562-lose-testbase into lp:~go-bot/juju-core/trunk

  1. 562-lose-testbase
  2. Merge into trunk
Proposed by Roger Peppe
Status: Merged
Approved by: Roger Peppe
Approved revision: no longer in the source branch.
Merged at revision: 2818
Proposed branch: lp:~rogpeppe/juju-core/562-lose-testbase
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 472 lines (+48/-181)
13 files modified
agent/tools/tools_test.go (+1/-2)
cmd/args_test.go (+2/-2)
cmd/package_test.go (+6/-5)
cmd/supercommand_test.go (+2/-2)
dependencies.tsv (+1/-1)
environs/sync/sync_test.go (+15/-2)
juju/osenv/package_test.go (+2/-2)
state/api/rsyslog/rsyslog_test.go (+1/-1)
testing/base.go (+13/-4)
testing/imports.go (+5/-42)
testing/testbase/log.go (+0/-39)
testing/testbase/log_test.go (+0/-53)
testing/testbase/package_test.go (+0/-26)
To merge this branch: bzr merge lp:~rogpeppe/juju-core/562-lose-testbase
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+221560@code.launchpad.net

Commit message

testing/testbase: remove

It's unnecessary - we move FindJujuCoreImports to
testing (and make it call the less specific version in
github.com/juju/testing).

We temporarily lose the ability to change the logging level
in tests, but that will be reinstated directly in LoggingSuite - it's
not crucial behaviour.

Eventually most tests will use testing.IsolationSuite,
but in the meantime, add testing.CleanupSuite to
those tests that need it.

https://codereview.appspot.com/99670045/

Description of the change

testing/testbase: remove

It's unnecessary - we move FindJujuCoreImports to
testing (and make it call the less specific version in
github.com/juju/testing).

We temporarily lose the ability to change the logging level
in tests, but that will be reinstated directly in LoggingSuite - it's
not crucial behaviour.

Eventually most tests will use testing.IsolationSuite,
but in the meantime, add testing.CleanupSuite to
those tests that need it.

https://codereview.appspot.com/99670045/

To post a comment you must log in.
Revision history for this message
Roger Peppe (rogpeppe) wrote :

Reviewers: mp+221560_code.launchpad.net,

Message:
Please take a look.

Description:
testing/testbase: remove

It's unnecessary - we move FindJujuCoreImports to
testing (and make it call the less specific version in
github.com/juju/testing).

We temporarily lose the ability to change the logging level
in tests, but that will be reinstated directly in LoggingSuite - it's
not crucial behaviour.

Eventually most tests will use testing.IsolationSuite,
but in the meantime, add testing.CleanupSuite to
those tests that need it.

https://code.launchpad.net/~rogpeppe/juju-core/562-lose-testbase/+merge/221560

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/99670045/

Affected files (+59, -189 lines):
   A [revision details]
   M agent/tools/tools_test.go
   M cmd/args_test.go
   M cmd/package_test.go
   M cmd/supercommand_test.go
   M dependencies.tsv
   M environs/sync/sync_test.go
   M juju/osenv/package_test.go
   M state/api/rsyslog/rsyslog_test.go
   M testing/base.go
   M testing/imports.go
   D testing/testbase/log.go
   D testing/testbase/log_test.go
   D testing/testbase/package_test.go
   M utils/exec/package_test.go
   M utils/fslock/package_test.go
   M utils/home_windows_test.go
   M utils/package_test.go

Revision history for this message
Roger Peppe (rogpeppe) wrote :

Please take a look.

https://codereview.appspot.com/99670045/

Revision history for this message
Francesco Banconi (frankban) wrote :

Thanks for this branch Roger!
LGTM with some minor/questions below.

https://codereview.appspot.com/99670045/diff/40001/cmd/package_test.go
File cmd/package_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/cmd/package_test.go#newcode26
cmd/package_test.go:26: jc.DeepEquals,
So I presume github.com/juju/testing/checkers.DeepEquals is preferred
over launchpad.net/gocheck.DeepEquals.

https://codereview.appspot.com/99670045/diff/40001/cmd/supercommand_test.go
File cmd/supercommand_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/cmd/supercommand_test.go#newcode11
cmd/supercommand_test.go:11: gitjujutesting "github.com/juju/testing"
We use just "github.com/juju/testing" and coretesting for
"launchpad.net/juju-core/testing" elsewhere.
Anyway this makes the diff short and clean, so <shrug>.
Do we have conventions for import names?

https://codereview.appspot.com/99670045/diff/40001/cmd/supercommand_test.go#newcode28
cmd/supercommand_test.go:28: gitjujutesting.IsolationSuite
Cool.

https://codereview.appspot.com/99670045/diff/40001/testing/base.go
File testing/base.go (right):

https://codereview.appspot.com/99670045/diff/40001/testing/base.go#newcode33
testing/base.go:33: t.CleanupSuite.SetUpSuite(c)
In the Isolation suite we use the reversed order:
s.CleanupSuite.SetUpSuite(c)
s.LoggingSuite.SetUpSuite(c)
I don't think in this case it makes so much difference, but keeping them
in sync now can avoid some confusion in the future.
What do you think?
This also applies to sync_test.

https://codereview.appspot.com/99670045/diff/40001/testing/imports.go
File testing/imports.go (right):

https://codereview.appspot.com/99670045/diff/40001/testing/imports.go#newcode17
testing/imports.go:17: imps, err := testing.FindImports(packageName,
jujuPkgPrefix)
Nice.

https://codereview.appspot.com/99670045/diff/40001/utils/exec/package_test.go
File utils/exec/package_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/utils/exec/package_test.go#newcode7
utils/exec/package_test.go:7: stdtesting "testing"
This looks like an import error: stdtesting vs testing.

https://codereview.appspot.com/99670045/diff/40001/utils/fslock/package_test.go
File utils/fslock/package_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/utils/fslock/package_test.go#newcode7
utils/fslock/package_test.go:7: stdtesting "testing"
Ditto.

https://codereview.appspot.com/99670045/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

https://codereview.appspot.com/99670045/diff/40001/cmd/package_test.go
File cmd/package_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/cmd/package_test.go#newcode26
cmd/package_test.go:26: jc.DeepEquals,
On 2014/05/30 16:14:56, frankban wrote:
> So I presume github.com/juju/testing/checkers.DeepEquals is preferred
 over
> launchpad.net/gocheck.DeepEquals.

It gives better diagnostics on failure.

https://codereview.appspot.com/99670045/diff/40001/cmd/supercommand_test.go
File cmd/supercommand_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/cmd/supercommand_test.go#newcode11
cmd/supercommand_test.go:11: gitjujutesting "github.com/juju/testing"
On 2014/05/30 16:14:57, frankban wrote:
> We use just "github.com/juju/testing" and coretesting for
> "launchpad.net/juju-core/testing" elsewhere.
> Anyway this makes the diff short and clean, so <shrug>.
> Do we have conventions for import names?

not really. i asked about this on juju-dev, and no-one seemed that
bothered.

https://codereview.appspot.com/99670045/diff/40001/testing/base.go
File testing/base.go (right):

https://codereview.appspot.com/99670045/diff/40001/testing/base.go#newcode33
testing/base.go:33: t.CleanupSuite.SetUpSuite(c)
On 2014/05/30 16:14:57, frankban wrote:
> In the Isolation suite we use the reversed order:
> s.CleanupSuite.SetUpSuite(c)
> s.LoggingSuite.SetUpSuite(c)
> I don't think in this case it makes so much difference, but keeping
 them in sync
> now can avoid some confusion in the future.
> What do you think?
> This also applies to sync_test.

sgtm, will do.

https://codereview.appspot.com/99670045/diff/40001/utils/exec/package_test.go
File utils/exec/package_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/utils/exec/package_test.go#newcode7
utils/exec/package_test.go:7: stdtesting "testing"
On 2014/05/30 16:14:57, frankban wrote:
> This looks like an import error: stdtesting vs testing.

yup, i haven't resolved all the trunk merging issues. will fix.

https://codereview.appspot.com/99670045/

Revision history for this message
Wayne Witzel III (wwitzel3) wrote :

My feeling is, I don't care where and how things are aliased, but it
should be consistent so that when I am looking at a file and I see
"coretesting" and the next file I see "testing" .. I shouldn't have to
think that might be juju-core/testing, just sans alias.

Also I'm -1 with respect to alias of the standard lib testing to
stdtesting. That just inserts unneeded convention in to our code. We can
easily alias our other testing packages and not have to remember that
stdtesting is really just testing.

I think we should try to use the aliases coretesting, gitjujutesting,
statetesting, jujutesting, etc.. even if they aren't conflicting with
any other imported packages. That consistency will make our code a whole
lot easier to follow IMO.

https://codereview.appspot.com/99670045/diff/40001/cmd/package_test.go
File cmd/package_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/cmd/package_test.go#newcode7
cmd/package_test.go:7: stdtesting "testing"
Why alias the standard library testing package?

https://codereview.appspot.com/99670045/diff/40001/cmd/supercommand_test.go
File cmd/supercommand_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/cmd/supercommand_test.go#newcode16
cmd/supercommand_test.go:16: "launchpad.net/juju-core/testing"
Why not alias this to coretesting?

https://codereview.appspot.com/99670045/diff/40001/environs/sync/sync_test.go
File environs/sync/sync_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/environs/sync/sync_test.go#newcode15
environs/sync/sync_test.go:15: "testing"
Same vein. Here we don't alias to stdtesting.

https://codereview.appspot.com/99670045/diff/40001/environs/sync/sync_test.go#newcode32
environs/sync/sync_test.go:32: coretesting
"launchpad.net/juju-core/testing"
Again, here we do alias to coretesting.

https://codereview.appspot.com/99670045/

Revision history for this message
Wayne Witzel III (wwitzel3) wrote :

On 2014/05/30 16:30:53, wwitzel3 wrote:
> My feeling is, I don't care where and how things are aliased, but it
 should be
> consistent so that when I am looking at a file and I see "coretesting"
 and the
> next file I see "testing" .. I shouldn't have to think that might be
> juju-core/testing, just sans alias.

> Also I'm -1 with respect to alias of the standard lib testing to
stdtesting.
> That just inserts unneeded convention in to our code. We can easily
alias our
> other testing packages and not have to remember that stdtesting is
really just
> testing.

> I think we should try to use the aliases coretesting, gitjujutesting,
> statetesting, jujutesting, etc.. even if they aren't conflicting with
any other
> imported packages. That consistency will make our code a whole lot
easier to
> follow IMO.

> https://codereview.appspot.com/99670045/diff/40001/cmd/package_test.go
> File cmd/package_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/cmd/package_test.go#newcode7
> cmd/package_test.go:7: stdtesting "testing"
> Why alias the standard library testing package?

https://codereview.appspot.com/99670045/diff/40001/cmd/supercommand_test.go
> File cmd/supercommand_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/cmd/supercommand_test.go#newcode16
> cmd/supercommand_test.go:16: "launchpad.net/juju-core/testing"
> Why not alias this to coretesting?

https://codereview.appspot.com/99670045/diff/40001/environs/sync/sync_test.go
> File environs/sync/sync_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/environs/sync/sync_test.go#newcode15
> environs/sync/sync_test.go:15: "testing"
> Same vein. Here we don't alias to stdtesting.

https://codereview.appspot.com/99670045/diff/40001/environs/sync/sync_test.go#newcode32
> environs/sync/sync_test.go:32: coretesting
 "launchpad.net/juju-core/testing"
> Again, here we do alias to coretesting.

And by "how" I meant, naming wise.

https://codereview.appspot.com/99670045/

Revision history for this message
John A Meinel (jameinel) wrote :

I'm pretty sure Tim makes active use of the "run the test suite with a different log level", though I think he's the only one that I know of that does.

It would be good to at least keep him up to date on the change and where it is headed.

I do agree with the idea from William that it will be easier to understand test suites if we use common naming for the actual test packages. Though it really starts to sound like we should just be naming the packages like that, rather than forcing everyone to do the aliasing.

Especially "testing" because it is reasonably likely that you might need more than one of those in a new test suite.

Revision history for this message
Roger Peppe (rogpeppe) wrote :

On 30 May 2014 17:30, <email address hidden> wrote:
> My feeling is, I don't care where and how things are aliased, but it
> should be consistent so that when I am looking at a file and I see
> "coretesting" and the next file I see "testing" .. I shouldn't have to
> think that might be juju-core/testing, just sans alias.
>
> Also I'm -1 with respect to alias of the standard lib testing to
> stdtesting. That just inserts unneeded convention in to our code. We can
> easily alias our other testing packages and not have to remember that
> stdtesting is really just testing.
>
> I think we should try to use the aliases coretesting, gitjujutesting,
> statetesting, jujutesting, etc.. even if they aren't conflicting with
> any other imported packages. That consistency will make our code a whole
> lot easier to follow IMO.

Perhaps you're right, but that's not been the convention up until now.
If we do decide to do that, perhaps we should just rename those packages so
we don't need the identifier.

Revision history for this message
Roger Peppe (rogpeppe) wrote :

Please take a look.

https://codereview.appspot.com/99670045/diff/40001/cmd/supercommand_test.go
File cmd/supercommand_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/cmd/supercommand_test.go#newcode16
cmd/supercommand_test.go:16: "launchpad.net/juju-core/testing"
On 2014/05/30 16:30:52, wwitzel3 wrote:
> Why not alias this to coretesting?

The original rationale was that juju-core/testing feels "native" so
earns the right to be unaliased when reasonable. I've left it that way
here to save churn, although it could easily be coretesting too.

https://codereview.appspot.com/99670045/diff/40001/environs/sync/sync_test.go
File environs/sync/sync_test.go (right):

https://codereview.appspot.com/99670045/diff/40001/environs/sync/sync_test.go#newcode15
environs/sync/sync_test.go:15: "testing"
On 2014/05/30 16:30:52, wwitzel3 wrote:
> Same vein. Here we don't alias to stdtesting.

Yeah, conventionally we would alias to stdtesting here
(after all, the identifier is only used once)

https://codereview.appspot.com/99670045/diff/40001/testing/base.go
File testing/base.go (right):

https://codereview.appspot.com/99670045/diff/40001/testing/base.go#newcode33
testing/base.go:33: t.CleanupSuite.SetUpSuite(c)
On 2014/05/30 16:19:40, rog wrote:
> On 2014/05/30 16:14:57, frankban wrote:
> > In the Isolation suite we use the reversed order:
> > s.CleanupSuite.SetUpSuite(c)
> > s.LoggingSuite.SetUpSuite(c)
> > I don't think in this case it makes so much difference, but keeping
 them in
> sync
> > now can avoid some confusion in the future.
> > What do you think?
> > This also applies to sync_test.

> sgtm, will do.

Done.

https://codereview.appspot.com/99670045/

Revision history for this message
Go Bot (go-bot) wrote :

The attempt to merge lp:~rogpeppe/juju-core/562-lose-testbase into lp:juju-core failed. Below is the output from the failed tests.

godeps: cannot update "/home/tarmac/trees/src/github.com/juju/testing": fatal: reference is not a tree: 77f66ce9a8a203fd6af74b38fb207666d5cdeae6
mongod: no process found

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'agent/tools/tools_test.go'
--- agent/tools/tools_test.go 2014-05-20 04:27:02 +0000
+++ agent/tools/tools_test.go 2014-06-02 13:47:12 +0000
@@ -15,7 +15,6 @@
1515
16 agenttools "launchpad.net/juju-core/agent/tools"16 agenttools "launchpad.net/juju-core/agent/tools"
17 "launchpad.net/juju-core/testing"17 "launchpad.net/juju-core/testing"
18 "launchpad.net/juju-core/testing/testbase"
19 coretest "launchpad.net/juju-core/tools"18 coretest "launchpad.net/juju-core/tools"
20 "launchpad.net/juju-core/version"19 "launchpad.net/juju-core/version"
21)20)
@@ -37,7 +36,7 @@
37 // or any of the other bigger packages that'll drag in yet more dependencies.36 // or any of the other bigger packages that'll drag in yet more dependencies.
38 // Only imports that start with "launchpad.net/juju-core" are checked, and the37 // Only imports that start with "launchpad.net/juju-core" are checked, and the
39 // resulting slice has that prefix removed to keep the output short.38 // resulting slice has that prefix removed to keep the output short.
40 c.Assert(testbase.FindJujuCoreImports(c, "launchpad.net/juju-core/agent/tools"),39 c.Assert(testing.FindJujuCoreImports(c, "launchpad.net/juju-core/agent/tools"),
41 gc.DeepEquals,40 gc.DeepEquals,
42 []string{"juju/arch", "tools", "utils/set", "version"})41 []string{"juju/arch", "tools", "utils/set", "version"})
43}42}
4443
=== modified file 'cmd/args_test.go'
--- cmd/args_test.go 2014-04-11 01:04:15 +0000
+++ cmd/args_test.go 2014-06-02 13:47:12 +0000
@@ -7,15 +7,15 @@
7 "fmt"7 "fmt"
8 "io/ioutil"8 "io/ioutil"
99
10 "github.com/juju/testing"
10 "launchpad.net/gnuflag"11 "launchpad.net/gnuflag"
11 gc "launchpad.net/gocheck"12 gc "launchpad.net/gocheck"
1213
13 "launchpad.net/juju-core/cmd"14 "launchpad.net/juju-core/cmd"
14 "launchpad.net/juju-core/testing/testbase"
15)15)
1616
17type ArgsSuite struct {17type ArgsSuite struct {
18 testbase.LoggingSuite18 testing.LoggingSuite
19}19}
2020
21var _ = gc.Suite(&ArgsSuite{})21var _ = gc.Suite(&ArgsSuite{})
2222
=== modified file 'cmd/package_test.go'
--- cmd/package_test.go 2014-05-21 02:34:39 +0000
+++ cmd/package_test.go 2014-06-02 13:47:12 +0000
@@ -4,14 +4,15 @@
4package cmd_test4package cmd_test
55
6import (6import (
7 "testing"7 stdtesting "testing"
88
9 jc "github.com/juju/testing/checkers"
9 gc "launchpad.net/gocheck"10 gc "launchpad.net/gocheck"
1011
11 "launchpad.net/juju-core/testing/testbase"12 coretesting "launchpad.net/juju-core/testing"
12)13)
1314
14func Test(t *testing.T) { gc.TestingT(t) }15func Test(t *stdtesting.T) { gc.TestingT(t) }
1516
16type Dependencies struct{}17type Dependencies struct{}
1718
@@ -21,7 +22,7 @@
21 // This test is to ensure we don't bring in dependencies without thinking.22 // This test is to ensure we don't bring in dependencies without thinking.
22 // Looking at the "environs/config", it is just for JujuHome. This should23 // Looking at the "environs/config", it is just for JujuHome. This should
23 // really be moved into "juju/osenv".24 // really be moved into "juju/osenv".
24 c.Assert(testbase.FindJujuCoreImports(c, "launchpad.net/juju-core/cmd"),25 c.Assert(coretesting.FindJujuCoreImports(c, "launchpad.net/juju-core/cmd"),
25 gc.DeepEquals,26 jc.DeepEquals,
26 []string{"juju/arch", "juju/osenv", "names", "utils", "version"})27 []string{"juju/arch", "juju/osenv", "names", "utils", "version"})
27}28}
2829
=== modified file 'cmd/supercommand_test.go'
--- cmd/supercommand_test.go 2014-05-26 04:33:18 +0000
+++ cmd/supercommand_test.go 2014-06-02 13:47:12 +0000
@@ -8,12 +8,12 @@
8 "fmt"8 "fmt"
9 "strings"9 "strings"
1010
11 gitjujutesting "github.com/juju/testing"
11 "launchpad.net/gnuflag"12 "launchpad.net/gnuflag"
12 gc "launchpad.net/gocheck"13 gc "launchpad.net/gocheck"
1314
14 "launchpad.net/juju-core/cmd"15 "launchpad.net/juju-core/cmd"
15 "launchpad.net/juju-core/testing"16 "launchpad.net/juju-core/testing"
16 "launchpad.net/juju-core/testing/testbase"
17 "launchpad.net/juju-core/version"17 "launchpad.net/juju-core/version"
18)18)
1919
@@ -25,7 +25,7 @@
25}25}
2626
27type SuperCommandSuite struct {27type SuperCommandSuite struct {
28 testbase.LoggingSuite28 gitjujutesting.IsolationSuite
29}29}
3030
31var _ = gc.Suite(&SuperCommandSuite{})31var _ = gc.Suite(&SuperCommandSuite{})
3232
=== modified file 'dependencies.tsv'
--- dependencies.tsv 2014-05-29 20:05:19 +0000
+++ dependencies.tsv 2014-06-02 13:47:12 +0000
@@ -11,7 +11,7 @@
11github.com/juju/errors git 075df0417dbcc39d24ee18248d2f8d6e3eed598b 11github.com/juju/errors git 075df0417dbcc39d24ee18248d2f8d6e3eed598b
12github.com/juju/loggo git 7e8c70b24b80b95b2284f09306aac0bd93588db8 12github.com/juju/loggo git 7e8c70b24b80b95b2284f09306aac0bd93588db8
13github.com/juju/ratelimit git 0025ab75db6c6eaa4ffff0240c2c9e617ad1a0eb 13github.com/juju/ratelimit git 0025ab75db6c6eaa4ffff0240c2c9e617ad1a0eb
14github.com/juju/testing git 0f4fb4f667cdc8e1a6c763434f7b486e2256df6a 14github.com/juju/testing git 77f66ce9a8a203fd6af74b38fb207666d5cdeae6
15labix.org/v2/mgo bzr gustavo@niemeyer.net-20140331185009-fhnh3xzfdpicup0j 27315labix.org/v2/mgo bzr gustavo@niemeyer.net-20140331185009-fhnh3xzfdpicup0j 273
16launchpad.net/gnuflag bzr roger.peppe@canonical.com-20121003093437-zcyyw0lpvj2nifpk 1216launchpad.net/gnuflag bzr roger.peppe@canonical.com-20121003093437-zcyyw0lpvj2nifpk 12
17launchpad.net/goamz bzr roger.peppe@canonical.com-20131218155244-hbnkvlkkzy3vmlh9 4417launchpad.net/goamz bzr roger.peppe@canonical.com-20131218155244-hbnkvlkkzy3vmlh9 44
1818
=== modified file 'environs/sync/sync_test.go'
--- environs/sync/sync_test.go 2014-05-20 00:01:25 +0000
+++ environs/sync/sync_test.go 2014-06-02 13:47:12 +0000
@@ -14,6 +14,7 @@
14 "sort"14 "sort"
15 "testing"15 "testing"
1616
17 gitjujutesting "github.com/juju/testing"
17 jc "github.com/juju/testing/checkers"18 jc "github.com/juju/testing/checkers"
18 gc "launchpad.net/gocheck"19 gc "launchpad.net/gocheck"
1920
@@ -29,7 +30,6 @@
29 toolstesting "launchpad.net/juju-core/environs/tools/testing"30 toolstesting "launchpad.net/juju-core/environs/tools/testing"
30 "launchpad.net/juju-core/provider/dummy"31 "launchpad.net/juju-core/provider/dummy"
31 coretesting "launchpad.net/juju-core/testing"32 coretesting "launchpad.net/juju-core/testing"
32 "launchpad.net/juju-core/testing/testbase"
33 coretools "launchpad.net/juju-core/tools"33 coretools "launchpad.net/juju-core/tools"
34 "launchpad.net/juju-core/utils"34 "launchpad.net/juju-core/utils"
35 "launchpad.net/juju-core/version"35 "launchpad.net/juju-core/version"
@@ -408,7 +408,8 @@
408408
409type badBuildSuite struct {409type badBuildSuite struct {
410 env environs.Environ410 env environs.Environ
411 testbase.LoggingSuite411 gitjujutesting.LoggingSuite
412 gitjujutesting.CleanupSuite
412 envtesting.ToolsFixture413 envtesting.ToolsFixture
413}414}
414415
@@ -417,7 +418,18 @@
417exit 1418exit 1
418`[1:]419`[1:]
419420
421func (s *badBuildSuite) SetUpSuite(c *gc.C) {
422 s.CleanupSuite.SetUpSuite(c)
423 s.LoggingSuite.SetUpSuite(c)
424}
425
426func (s *badBuildSuite) TearDownSuite(c *gc.C) {
427 s.LoggingSuite.TearDownSuite(c)
428 s.CleanupSuite.TearDownSuite(c)
429}
430
420func (s *badBuildSuite) SetUpTest(c *gc.C) {431func (s *badBuildSuite) SetUpTest(c *gc.C) {
432 s.CleanupSuite.SetUpTest(c)
421 s.LoggingSuite.SetUpTest(c)433 s.LoggingSuite.SetUpTest(c)
422 s.ToolsFixture.SetUpTest(c)434 s.ToolsFixture.SetUpTest(c)
423 // We only want to use simplestreams to find any synced tools.435 // We only want to use simplestreams to find any synced tools.
@@ -443,6 +455,7 @@
443 dummy.Reset()455 dummy.Reset()
444 s.ToolsFixture.TearDownTest(c)456 s.ToolsFixture.TearDownTest(c)
445 s.LoggingSuite.TearDownTest(c)457 s.LoggingSuite.TearDownTest(c)
458 s.CleanupSuite.TearDownTest(c)
446}459}
447460
448func (s *badBuildSuite) TestBundleToolsBadBuild(c *gc.C) {461func (s *badBuildSuite) TestBundleToolsBadBuild(c *gc.C) {
449462
=== modified file 'juju/osenv/package_test.go'
--- juju/osenv/package_test.go 2014-05-26 08:49:54 +0000
+++ juju/osenv/package_test.go 2014-06-02 13:47:12 +0000
@@ -9,7 +9,7 @@
9 "github.com/juju/testing"9 "github.com/juju/testing"
10 gc "launchpad.net/gocheck"10 gc "launchpad.net/gocheck"
1111
12 "launchpad.net/juju-core/testing/testbase"12 coretesting "launchpad.net/juju-core/testing"
13)13)
1414
15func Test(t *stdtesting.T) {15func Test(t *stdtesting.T) {
@@ -24,6 +24,6 @@
2424
25// TODO (frankban): remove this test once juju-core/utils is on Github.25// TODO (frankban): remove this test once juju-core/utils is on Github.
26func (*importSuite) TestTemporaryDependencies(c *gc.C) {26func (*importSuite) TestTemporaryDependencies(c *gc.C) {
27 c.Assert(testbase.FindJujuCoreImports(c, "launchpad.net/juju-core/juju/osenv"),27 c.Assert(coretesting.FindJujuCoreImports(c, "launchpad.net/juju-core/juju/osenv"),
28 gc.DeepEquals, []string{"utils"})28 gc.DeepEquals, []string{"utils"})
29}29}
3030
=== modified file 'state/api/rsyslog/rsyslog_test.go'
--- state/api/rsyslog/rsyslog_test.go 2014-05-22 15:23:46 +0000
+++ state/api/rsyslog/rsyslog_test.go 2014-06-02 13:47:12 +0000
@@ -5,12 +5,12 @@
55
6import (6import (
7 gc "launchpad.net/gocheck"7 gc "launchpad.net/gocheck"
8
8 "launchpad.net/juju-core/instance"9 "launchpad.net/juju-core/instance"
9 "launchpad.net/juju-core/juju/testing"10 "launchpad.net/juju-core/juju/testing"
10 "launchpad.net/juju-core/state"11 "launchpad.net/juju-core/state"
11 "launchpad.net/juju-core/state/api"12 "launchpad.net/juju-core/state/api"
12 "launchpad.net/juju-core/state/api/rsyslog"13 "launchpad.net/juju-core/state/api/rsyslog"
13
14 statetesting "launchpad.net/juju-core/state/testing"14 statetesting "launchpad.net/juju-core/state/testing"
15 coretesting "launchpad.net/juju-core/testing"15 coretesting "launchpad.net/juju-core/testing"
16)16)
1717
=== modified file 'testing/base.go'
--- testing/base.go 2014-05-23 12:58:44 +0000
+++ testing/base.go 2014-06-02 13:47:12 +0000
@@ -8,10 +8,10 @@
8 "os"8 "os"
9 "path/filepath"9 "path/filepath"
1010
11 "github.com/juju/testing"
11 gc "launchpad.net/gocheck"12 gc "launchpad.net/gocheck"
1213
13 "launchpad.net/juju-core/juju/osenv"14 "launchpad.net/juju-core/juju/osenv"
14 "launchpad.net/juju-core/testing/testbase"
15 "launchpad.net/juju-core/utils"15 "launchpad.net/juju-core/utils"
16)16)
1717
@@ -22,17 +22,26 @@
22// - protection of user's home directory22// - protection of user's home directory
23// - scrubbing of env vars23// - scrubbing of env vars
24type BaseSuite struct {24type BaseSuite struct {
25 testbase.LoggingSuite25 testing.CleanupSuite
26 testing.LoggingSuite
26 oldHomeEnv string27 oldHomeEnv string
27 oldEnvironment map[string]string28 oldEnvironment map[string]string
28}29}
2930
30func (t *BaseSuite) SetUpSuite(c *gc.C) {31func (t *BaseSuite) SetUpSuite(c *gc.C) {
32 t.CleanupSuite.SetUpSuite(c)
31 t.LoggingSuite.SetUpSuite(c)33 t.LoggingSuite.SetUpSuite(c)
32 t.PatchValue(&utils.OutgoingAccessAllowed, false)34 t.PatchValue(&utils.OutgoingAccessAllowed, false)
33}35}
3436
37func (t *BaseSuite) TearDownSuite(c *gc.C) {
38 t.LoggingSuite.TearDownSuite(c)
39 t.CleanupSuite.TearDownSuite(c)
40}
41
35func (t *BaseSuite) SetUpTest(c *gc.C) {42func (t *BaseSuite) SetUpTest(c *gc.C) {
43 t.CleanupSuite.SetUpTest(c)
44 t.LoggingSuite.SetUpTest(c)
36 t.oldEnvironment = make(map[string]string)45 t.oldEnvironment = make(map[string]string)
37 for _, name := range []string{46 for _, name := range []string{
38 osenv.JujuHomeEnvKey,47 osenv.JujuHomeEnvKey,
@@ -46,15 +55,15 @@
46 os.Setenv(osenv.JujuHomeEnvKey, "")55 os.Setenv(osenv.JujuHomeEnvKey, "")
47 os.Setenv(osenv.JujuEnvEnvKey, "")56 os.Setenv(osenv.JujuEnvEnvKey, "")
48 os.Setenv(osenv.JujuLoggingConfigEnvKey, "")57 os.Setenv(osenv.JujuLoggingConfigEnvKey, "")
49 t.LoggingSuite.SetUpTest(c)
50}58}
5159
52func (t *BaseSuite) TearDownTest(c *gc.C) {60func (t *BaseSuite) TearDownTest(c *gc.C) {
53 t.LoggingSuite.TearDownTest(c)
54 for name, value := range t.oldEnvironment {61 for name, value := range t.oldEnvironment {
55 os.Setenv(name, value)62 os.Setenv(name, value)
56 }63 }
57 utils.SetHome(t.oldHomeEnv)64 utils.SetHome(t.oldHomeEnv)
65 t.LoggingSuite.TearDownTest(c)
66 t.CleanupSuite.TearDownTest(c)
58}67}
5968
60type TestFile struct {69type TestFile struct {
6170
=== renamed file 'testing/testbase/imports.go' => 'testing/imports.go'
--- testing/testbase/imports.go 2014-01-21 21:29:02 +0000
+++ testing/imports.go 2014-06-02 13:47:12 +0000
@@ -1,14 +1,10 @@
1// Copyright 2013 Canonical Ltd.1// Copyright 2013 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.2// Licensed under the AGPLv3, see LICENCE file for details.
33
4package testbase4package testing
55
6import (6import (
7 "go/build"7 "github.com/juju/testing"
8 "path/filepath"
9 "sort"
10 "strings"
11
12 gc "launchpad.net/gocheck"8 gc "launchpad.net/gocheck"
13)9)
1410
@@ -18,40 +14,7 @@
18// imported by the packageName parameter. The resulting list removes the14// imported by the packageName parameter. The resulting list removes the
19// common prefix "launchpad.net/juju-core/" leaving just the short names.15// common prefix "launchpad.net/juju-core/" leaving just the short names.
20func FindJujuCoreImports(c *gc.C, packageName string) []string {16func FindJujuCoreImports(c *gc.C, packageName string) []string {
21 var result []string17 imps, err := testing.FindImports(packageName, jujuPkgPrefix)
22 allpkgs := make(map[string]bool)18 c.Assert(err, gc.IsNil)
2319 return imps
24 findJujuCoreImports(c, packageName, allpkgs)
25 for name := range allpkgs {
26 result = append(result, name[len(jujuPkgPrefix):])
27 }
28 sort.Strings(result)
29 return result
30}
31
32// findJujuCoreImports recursively adds all imported packages of given
33// package (packageName) to allpkgs map.
34func findJujuCoreImports(c *gc.C, packageName string, allpkgs map[string]bool) {
35
36 var imports []string
37
38 for _, root := range build.Default.SrcDirs() {
39 fullpath := filepath.Join(root, packageName)
40 pkg, err := build.ImportDir(fullpath, 0)
41 if err == nil {
42 imports = pkg.Imports
43 break
44 }
45 }
46 if imports == nil {
47 c.Fatalf(packageName + " not found")
48 }
49
50 for _, name := range imports {
51 if strings.HasPrefix(name, jujuPkgPrefix) {
52 allpkgs[name] = true
53 findJujuCoreImports(c, name, allpkgs)
54 }
55 }
56
57}20}
5821
=== removed directory 'testing/testbase'
=== removed file 'testing/testbase/log.go'
--- testing/testbase/log.go 2014-05-28 14:28:03 +0000
+++ testing/testbase/log.go 1970-01-01 00:00:00 +0000
@@ -1,39 +0,0 @@
1// Copyright 2012, 2013 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package testbase
5
6import (
7 "flag"
8
9 "github.com/juju/loggo"
10 "github.com/juju/testing"
11 gc "launchpad.net/gocheck"
12)
13
14// LoggingSuite redirects the juju logger to the test logger
15// when embedded in a gocheck suite type.
16type LoggingSuite struct {
17 testing.LoggingCleanupSuite
18}
19
20func (t *LoggingSuite) SetUpSuite(c *gc.C) {
21 t.LoggingCleanupSuite.SetUpSuite(c)
22 t.setUp(c)
23}
24
25func (t *LoggingSuite) SetUpTest(c *gc.C) {
26 t.LoggingCleanupSuite.SetUpTest(c)
27 t.PatchEnvironment("JUJU_LOGGING_CONFIG", "")
28 t.setUp(c)
29}
30
31var logConfig = flag.String("juju.log", "DEBUG", "logging configuration (see http://godoc.org/github.com/juju/loggo#ConfigureLoggers; also accepts a bare log level to configure the log level of the root module")
32
33func (t *LoggingSuite) setUp(c *gc.C) {
34 if _, ok := loggo.ParseLevel(*logConfig); ok {
35 *logConfig = "<root>=" + *logConfig
36 }
37 err := loggo.ConfigureLoggers(*logConfig)
38 c.Assert(err, gc.IsNil)
39}
400
=== removed file 'testing/testbase/log_test.go'
--- testing/testbase/log_test.go 2014-04-17 03:34:43 +0000
+++ testing/testbase/log_test.go 1970-01-01 00:00:00 +0000
@@ -1,53 +0,0 @@
1// Copyright 2013 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package testbase_test
5
6import (
7 "github.com/juju/loggo"
8 gc "launchpad.net/gocheck"
9
10 "launchpad.net/juju-core/testing/testbase"
11)
12
13var logger = loggo.GetLogger("juju.logsuite")
14
15var _ = gc.Suite(&logSuite{})
16
17type logSuite struct {
18 testbase.LoggingSuite
19}
20
21func (s *logSuite) SetUpSuite(c *gc.C) {
22 s.LoggingSuite.SetUpSuite(c)
23 logger.Infof("testing-SetUpSuite")
24 c.Assert(c.GetTestLog(), gc.Matches, ".*INFO juju.logsuite testing-SetUpSuite\n")
25}
26
27func (s *logSuite) TearDownSuite(c *gc.C) {
28 // Unfortunately there's no way of testing that the
29 // log output is printed, as the logger is printing
30 // a previously set up *gc.C. We print a message
31 // anyway so that we can manually verify it.
32 logger.Infof("testing-TearDownSuite")
33}
34
35func (s *logSuite) SetUpTest(c *gc.C) {
36 s.LoggingSuite.SetUpTest(c)
37 logger.Infof("testing-SetUpTest")
38 c.Assert(c.GetTestLog(), gc.Matches, ".*INFO juju.logsuite testing-SetUpTest\n")
39}
40
41func (s *logSuite) TearDownTest(c *gc.C) {
42 // The same applies here as to TearDownSuite.
43 logger.Infof("testing-TearDownTest")
44 s.LoggingSuite.TearDownTest(c)
45}
46
47func (s *logSuite) TestLog(c *gc.C) {
48 logger.Infof("testing-Test")
49 c.Assert(c.GetTestLog(), gc.Matches,
50 ".*INFO juju.logsuite testing-SetUpTest\n"+
51 ".*INFO juju.logsuite testing-Test\n",
52 )
53}
540
=== removed file 'testing/testbase/package_test.go'
--- testing/testbase/package_test.go 2013-09-20 02:10:46 +0000
+++ testing/testbase/package_test.go 1970-01-01 00:00:00 +0000
@@ -1,26 +0,0 @@
1// Copyright 2013 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package testbase_test
5
6import (
7 "testing"
8
9 gc "launchpad.net/gocheck"
10
11 "launchpad.net/juju-core/testing/testbase"
12)
13
14func Test(t *testing.T) {
15 gc.TestingT(t)
16}
17
18type DependencySuite struct{}
19
20var _ = gc.Suite(&DependencySuite{})
21
22func (*DependencySuite) TestPackageDependencies(c *gc.C) {
23 // This test is to ensure we don't bring in any juju-core dependencies.
24 c.Assert(testbase.FindJujuCoreImports(c, "launchpad.net/juju-core/testing/testbase"),
25 gc.HasLen, 0)
26}

Subscribers

People subscribed via source and target branches

to status/vote changes: