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

[Roger Peppe]

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/

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/

[Roger Peppe (rogpeppe)]

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

[Roger Peppe (rogpeppe)]

Please take a look.

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

[Francesco Banconi (frankban)]

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/

[Roger Peppe (rogpeppe)]

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/

[Wayne Witzel III (wwitzel3)]

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/

[Wayne Witzel III (wwitzel3)]

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/

[John A Meinel (jameinel)]

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.

[Roger Peppe (rogpeppe)]

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.

[Roger Peppe (rogpeppe)]

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/

[Go Bot (go-bot)]

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

=== 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 @@
 
 	agenttools "launchpad.net/juju-core/agent/tools"
 	"launchpad.net/juju-core/testing"
-	"launchpad.net/juju-core/testing/testbase"
 	coretest "launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/version"
 )
@@ -37,7 +36,7 @@
 	// or any of the other bigger packages that'll drag in yet more dependencies.
 	// Only imports that start with "launchpad.net/juju-core" are checked, and the
 	// resulting slice has that prefix removed to keep the output short.
-	c.Assert(testbase.FindJujuCoreImports(c, "launchpad.net/juju-core/agent/tools"),
+	c.Assert(testing.FindJujuCoreImports(c, "launchpad.net/juju-core/agent/tools"),
 		gc.DeepEquals,
 		[]string{"juju/arch", "tools", "utils/set", "version"})
 }

=== 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 @@
 	"fmt"
 	"io/ioutil"
 
+	"github.com/juju/testing"
 	"launchpad.net/gnuflag"
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/cmd"
-	"launchpad.net/juju-core/testing/testbase"
 )
 
 type ArgsSuite struct {
-	testbase.LoggingSuite
+	testing.LoggingSuite
 }
 
 var _ = gc.Suite(&ArgsSuite{})

=== 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 @@
 package cmd_test
 
 import (
-	"testing"
+	stdtesting "testing"
 
+	jc "github.com/juju/testing/checkers"
 	gc "launchpad.net/gocheck"
 
-	"launchpad.net/juju-core/testing/testbase"
+	coretesting "launchpad.net/juju-core/testing"
 )
 
-func Test(t *testing.T) { gc.TestingT(t) }
+func Test(t *stdtesting.T) { gc.TestingT(t) }
 
 type Dependencies struct{}
 
@@ -21,7 +22,7 @@
 	// This test is to ensure we don't bring in dependencies without thinking.
 	// Looking at the "environs/config", it is just for JujuHome.  This should
 	// really be moved into "juju/osenv".
-	c.Assert(testbase.FindJujuCoreImports(c, "launchpad.net/juju-core/cmd"),
-		gc.DeepEquals,
+	c.Assert(coretesting.FindJujuCoreImports(c, "launchpad.net/juju-core/cmd"),
+		jc.DeepEquals,
 		[]string{"juju/arch", "juju/osenv", "names", "utils", "version"})
 }

=== 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 @@
 	"fmt"
 	"strings"
 
+	gitjujutesting "github.com/juju/testing"
 	"launchpad.net/gnuflag"
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/cmd"
 	"launchpad.net/juju-core/testing"
-	"launchpad.net/juju-core/testing/testbase"
 	"launchpad.net/juju-core/version"
 )
 
@@ -25,7 +25,7 @@
 }
 
 type SuperCommandSuite struct {
-	testbase.LoggingSuite
+	gitjujutesting.IsolationSuite
 }
 
 var _ = gc.Suite(&SuperCommandSuite{})

=== 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 @@
 github.com/juju/errors	git	075df0417dbcc39d24ee18248d2f8d6e3eed598b	
 github.com/juju/loggo	git	7e8c70b24b80b95b2284f09306aac0bd93588db8	
 github.com/juju/ratelimit	git	0025ab75db6c6eaa4ffff0240c2c9e617ad1a0eb	
-github.com/juju/testing	git	0f4fb4f667cdc8e1a6c763434f7b486e2256df6a	
+github.com/juju/testing	git	77f66ce9a8a203fd6af74b38fb207666d5cdeae6	
 labix.org/v2/mgo	bzr	[e-mail]-20140331185009-fhnh3xzfdpicup0j	273
 launchpad.net/gnuflag	bzr	[e-mail]-20121003093437-zcyyw0lpvj2nifpk	12
 launchpad.net/goamz	bzr	[e-mail]-20131218155244-hbnkvlkkzy3vmlh9	44

=== 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 @@
 	"sort"
 	"testing"
 
+	gitjujutesting "github.com/juju/testing"
 	jc "github.com/juju/testing/checkers"
 	gc "launchpad.net/gocheck"
 
@@ -29,7 +30,6 @@
 	toolstesting "launchpad.net/juju-core/environs/tools/testing"
 	"launchpad.net/juju-core/provider/dummy"
 	coretesting "launchpad.net/juju-core/testing"
-	"launchpad.net/juju-core/testing/testbase"
 	coretools "launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/utils"
 	"launchpad.net/juju-core/version"
@@ -408,7 +408,8 @@
 
 type badBuildSuite struct {
 	env environs.Environ
-	testbase.LoggingSuite
+	gitjujutesting.LoggingSuite
+	gitjujutesting.CleanupSuite
 	envtesting.ToolsFixture
 }
 
@@ -417,7 +418,18 @@
 exit 1
 `[1:]
 
+func (s *badBuildSuite) SetUpSuite(c *gc.C) {
+	s.CleanupSuite.SetUpSuite(c)
+	s.LoggingSuite.SetUpSuite(c)
+}
+
+func (s *badBuildSuite) TearDownSuite(c *gc.C) {
+	s.LoggingSuite.TearDownSuite(c)
+	s.CleanupSuite.TearDownSuite(c)
+}
+
 func (s *badBuildSuite) SetUpTest(c *gc.C) {
+	s.CleanupSuite.SetUpTest(c)
 	s.LoggingSuite.SetUpTest(c)
 	s.ToolsFixture.SetUpTest(c)
 	// We only want to use simplestreams to find any synced tools.
@@ -443,6 +455,7 @@
 	dummy.Reset()
 	s.ToolsFixture.TearDownTest(c)
 	s.LoggingSuite.TearDownTest(c)
+	s.CleanupSuite.TearDownTest(c)
 }
 
 func (s *badBuildSuite) TestBundleToolsBadBuild(c *gc.C) {

=== 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 @@
 	"github.com/juju/testing"
 	gc "launchpad.net/gocheck"
 
-	"launchpad.net/juju-core/testing/testbase"
+	coretesting "launchpad.net/juju-core/testing"
 )
 
 func Test(t *stdtesting.T) {
@@ -24,6 +24,6 @@
 
 // TODO (frankban): remove this test once juju-core/utils is on Github.
 func (*importSuite) TestTemporaryDependencies(c *gc.C) {
-	c.Assert(testbase.FindJujuCoreImports(c, "launchpad.net/juju-core/juju/osenv"),
+	c.Assert(coretesting.FindJujuCoreImports(c, "launchpad.net/juju-core/juju/osenv"),
 		gc.DeepEquals, []string{"utils"})
 }

=== 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 @@
 
 import (
 	gc "launchpad.net/gocheck"
+
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju/testing"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/state/api/rsyslog"
-
 	statetesting "launchpad.net/juju-core/state/testing"
 	coretesting "launchpad.net/juju-core/testing"
 )

=== 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 @@
 	"os"
 	"path/filepath"
 
+	"github.com/juju/testing"
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/juju/osenv"
-	"launchpad.net/juju-core/testing/testbase"
 	"launchpad.net/juju-core/utils"
 )
 
@@ -22,17 +22,26 @@
 // - protection of user's home directory
 // - scrubbing of env vars
 type BaseSuite struct {
-	testbase.LoggingSuite
+	testing.CleanupSuite
+	testing.LoggingSuite
 	oldHomeEnv     string
 	oldEnvironment map[string]string
 }
 
 func (t *BaseSuite) SetUpSuite(c *gc.C) {
+	t.CleanupSuite.SetUpSuite(c)
 	t.LoggingSuite.SetUpSuite(c)
 	t.PatchValue(&utils.OutgoingAccessAllowed, false)
 }
 
+func (t *BaseSuite) TearDownSuite(c *gc.C) {
+	t.LoggingSuite.TearDownSuite(c)
+	t.CleanupSuite.TearDownSuite(c)
+}
+
 func (t *BaseSuite) SetUpTest(c *gc.C) {
+	t.CleanupSuite.SetUpTest(c)
+	t.LoggingSuite.SetUpTest(c)
 	t.oldEnvironment = make(map[string]string)
 	for _, name := range []string{
 		osenv.JujuHomeEnvKey,
@@ -46,15 +55,15 @@
 	os.Setenv(osenv.JujuHomeEnvKey, "")
 	os.Setenv(osenv.JujuEnvEnvKey, "")
 	os.Setenv(osenv.JujuLoggingConfigEnvKey, "")
-	t.LoggingSuite.SetUpTest(c)
 }
 
 func (t *BaseSuite) TearDownTest(c *gc.C) {
-	t.LoggingSuite.TearDownTest(c)
 	for name, value := range t.oldEnvironment {
 		os.Setenv(name, value)
 	}
 	utils.SetHome(t.oldHomeEnv)
+	t.LoggingSuite.TearDownTest(c)
+	t.CleanupSuite.TearDownTest(c)
 }
 
 type TestFile struct {

=== 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 @@
 // Copyright 2013 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
-package testbase
+package testing
 
 import (
-	"go/build"
-	"path/filepath"
-	"sort"
-	"strings"
-
+	"github.com/juju/testing"
 	gc "launchpad.net/gocheck"
 )
 
@@ -18,40 +14,7 @@
 // imported by the packageName parameter.  The resulting list removes the
 // common prefix "launchpad.net/juju-core/" leaving just the short names.
 func FindJujuCoreImports(c *gc.C, packageName string) []string {
-	var result []string
-	allpkgs := make(map[string]bool)
-
-	findJujuCoreImports(c, packageName, allpkgs)
-	for name := range allpkgs {
-		result = append(result, name[len(jujuPkgPrefix):])
-	}
-	sort.Strings(result)
-	return result
-}
-
-// findJujuCoreImports recursively adds all imported packages of given
-// package (packageName) to allpkgs map.
-func findJujuCoreImports(c *gc.C, packageName string, allpkgs map[string]bool) {
-
-	var imports []string
-
-	for _, root := range build.Default.SrcDirs() {
-		fullpath := filepath.Join(root, packageName)
-		pkg, err := build.ImportDir(fullpath, 0)
-		if err == nil {
-			imports = pkg.Imports
-			break
-		}
-	}
-	if imports == nil {
-		c.Fatalf(packageName + " not found")
-	}
-
-	for _, name := range imports {
-		if strings.HasPrefix(name, jujuPkgPrefix) {
-			allpkgs[name] = true
-			findJujuCoreImports(c, name, allpkgs)
-		}
-	}
-
+	imps, err := testing.FindImports(packageName, jujuPkgPrefix)
+	c.Assert(err, gc.IsNil)
+	return imps
 }

=== 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 @@
-// Copyright 2012, 2013 Canonical Ltd.
-// Licensed under the AGPLv3, see LICENCE file for details.
-
-package testbase
-
-import (
-	"flag"
-
-	"github.com/juju/loggo"
-	"github.com/juju/testing"
-	gc "launchpad.net/gocheck"
-)
-
-// LoggingSuite redirects the juju logger to the test logger
-// when embedded in a gocheck suite type.
-type LoggingSuite struct {
-	testing.LoggingCleanupSuite
-}
-
-func (t *LoggingSuite) SetUpSuite(c *gc.C) {
-	t.LoggingCleanupSuite.SetUpSuite(c)
-	t.setUp(c)
-}
-
-func (t *LoggingSuite) SetUpTest(c *gc.C) {
-	t.LoggingCleanupSuite.SetUpTest(c)
-	t.PatchEnvironment("JUJU_LOGGING_CONFIG", "")
-	t.setUp(c)
-}
-
-var 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")
-
-func (t *LoggingSuite) setUp(c *gc.C) {
-	if _, ok := loggo.ParseLevel(*logConfig); ok {
-		*logConfig = "<root>=" + *logConfig
-	}
-	err := loggo.ConfigureLoggers(*logConfig)
-	c.Assert(err, gc.IsNil)
-}

=== 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 @@
-// Copyright 2013 Canonical Ltd.
-// Licensed under the AGPLv3, see LICENCE file for details.
-
-package testbase_test
-
-import (
-	"github.com/juju/loggo"
-	gc "launchpad.net/gocheck"
-
-	"launchpad.net/juju-core/testing/testbase"
-)
-
-var logger = loggo.GetLogger("juju.logsuite")
-
-var _ = gc.Suite(&logSuite{})
-
-type logSuite struct {
-	testbase.LoggingSuite
-}
-
-func (s *logSuite) SetUpSuite(c *gc.C) {
-	s.LoggingSuite.SetUpSuite(c)
-	logger.Infof("testing-SetUpSuite")
-	c.Assert(c.GetTestLog(), gc.Matches, ".*INFO juju.logsuite testing-SetUpSuite\n")
-}
-
-func (s *logSuite) TearDownSuite(c *gc.C) {
-	// Unfortunately there's no way of testing that the
-	// log output is printed, as the logger is printing
-	// a previously set up *gc.C. We print a message
-	// anyway so that we can manually verify it.
-	logger.Infof("testing-TearDownSuite")
-}
-
-func (s *logSuite) SetUpTest(c *gc.C) {
-	s.LoggingSuite.SetUpTest(c)
-	logger.Infof("testing-SetUpTest")
-	c.Assert(c.GetTestLog(), gc.Matches, ".*INFO juju.logsuite testing-SetUpTest\n")
-}
-
-func (s *logSuite) TearDownTest(c *gc.C) {
-	// The same applies here as to TearDownSuite.
-	logger.Infof("testing-TearDownTest")
-	s.LoggingSuite.TearDownTest(c)
-}
-
-func (s *logSuite) TestLog(c *gc.C) {
-	logger.Infof("testing-Test")
-	c.Assert(c.GetTestLog(), gc.Matches,
-		".*INFO juju.logsuite testing-SetUpTest\n"+
-			".*INFO juju.logsuite testing-Test\n",
-	)
-}

=== 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 @@
-// Copyright 2013 Canonical Ltd.
-// Licensed under the AGPLv3, see LICENCE file for details.
-
-package testbase_test
-
-import (
-	"testing"
-
-	gc "launchpad.net/gocheck"
-
-	"launchpad.net/juju-core/testing/testbase"
-)
-
-func Test(t *testing.T) {
-	gc.TestingT(t)
-}
-
-type DependencySuite struct{}
-
-var _ = gc.Suite(&DependencySuite{})
-
-func (*DependencySuite) TestPackageDependencies(c *gc.C) {
-	// This test is to ensure we don't bring in any juju-core dependencies.
-	c.Assert(testbase.FindJujuCoreImports(c, "launchpad.net/juju-core/testing/testbase"),
-		gc.HasLen, 0)
-}