Merge lp:~waigani/juju-core/1307241-isolate-from-jujud into lp:~go-bot/juju-core/trunk

[Jesse Meek]

Fix 1307241. Isolate jujud.

Add the following mock functions to
environs/tools/testing/testing.go:

GetMockBundleTools
GetMockBuildTools

Make envtools.BundleTools mockable.
Patch functions in tests where
needed to ensure tests pass without
dependencies on jujud.

https://codereview.appspot.com/87130045/

R=fwereade, wallyworld

Fix 1307241. Isolate jujud.

Add the following mock functions to
environs/tools/testing/testing.go:

GetMockUploadTools
GetMockBundleTools
GetMockBuildTools

Make envtools.BundleTools mockable.
Patch functions in tests where
needed to ensure tests pass without
dependencies on jujud.

https://codereview.appspot.com/87130045/

[Jesse Meek (waigani)]

Reviewers: mp+215590_code.launchpad.net,

Message:
Please take a look.

Description:
Fix 1307241 isolate tests

Lots of tests fail without the jujud package.
Isolate tests to ensure they can run without
jujud.

So far: patched sync.Upload in cmd/juju and
environs/bootstrap tests, tests now pass.

https://code.launchpad.net/~waigani/juju-core/1307241-isolate-from-jujud/+merge/215590

(do not edit description out of merge proposal)

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

Affected files (+107, -86 lines):
   A [revision details]
   M cmd/juju/bootstrap_test.go
   M cmd/juju/upgradejuju_test.go
   M environs/bootstrap/bootstrap_test.go
   M environs/jujutest/livetests.go
   M environs/sync/sync_test.go
   M environs/tools/build.go
   M environs/tools/testing/testing.go

[Dave Cheney (dave-cheney)]

On 2014/04/15 09:15:27, waigani wrote:
> Please take a look.

I'm not sure I understand the rational for this change. If jujud fails
to compile, maybe that is where the test should stop.

https://codereview.appspot.com/87130045/

[Ian Booth (wallyworld)]

On 2014/04/15 10:10:15, dfc wrote:
> On 2014/04/15 09:15:27, waigani wrote:
> > Please take a look.

> I'm not sure I understand the rational for this change. If jujud fails
to
> compile, maybe that is where the test should stop.

We want to mock out the BundleTools because it adds unnecessary
complexity to the tests which depend on it. eg when we test that tools
are synced, we don't care that the real compile is not done; we just
want a stub which runs quickly and is robust against whatever
arch/series/os it is running on. Having said that, there need to be
tests added which confirm that BundleTools itself works properly.

https://codereview.appspot.com/87130045/

[Ian Booth (wallyworld)]

Looking good so far. I like that duped functionality has been
consolidated.
If we are mocking out BundleTools, we need tests for BundleTools itself
to ensure that function does what it is supposed to do. We also need to
ensure (if not done already) that the tests which use the mock assert
that the mock is called correctly.

https://codereview.appspot.com/87130045/diff/20001/cmd/juju/upgradejuju_test.go
File cmd/juju/upgradejuju_test.go (right):

https://codereview.appspot.com/87130045/diff/20001/cmd/juju/upgradejuju_test.go#newcode393
cmd/juju/upgradejuju_test.go:393: ttesting.ToolsDir = c.MkDir()
Don't do this - make toolsDir an argument to GetMockBuildTools and pass
in s.toolsDir

https://codereview.appspot.com/87130045/diff/20001/environs/tools/testing/testing.go
File environs/tools/testing/testing.go (right):

https://codereview.appspot.com/87130045/diff/20001/environs/tools/testing/testing.go#newcode34
environs/tools/testing/testing.go:34: var ToolsDir string
remove this and make it an arg to the functions which need it

https://codereview.appspot.com/87130045/

[John A Meinel (jameinel)]

The tests mentioned fail with juju-mongodb because juju-mongodb doesn't provide a Javascript engine (juju itself doesn't use it, only the store subset does, and there only in tests, IIRC).

You need to have "mongodb-server" installed which provides the full V8 engine.

Though IMO, our test suite shouldn't require you to symlink juju-mongodb to be able to run, it should detect that /usr/bin/mongodb-server isn't available, and then it can skip the tests that depend on V8.

But yes, that is unrelated to your change.

[Jesse Meek (waigani)]

Please take a look.

https://codereview.appspot.com/87130045/

[William Reade (fwereade)]

I don't think this is quite there yet. Definitely some good stuff, but
we should chat.

https://codereview.appspot.com/87130045/diff/40001/environs/bootstrap/bootstrap_test.go
File environs/bootstrap/bootstrap_test.go (right):

https://codereview.appspot.com/87130045/diff/40001/environs/bootstrap/bootstrap_test.go#newcode24
environs/bootstrap/bootstrap_test.go:24: ttesting
"launchpad.net/juju-core/environs/tools/testing"
explicit "toolstesting" throughout please

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

https://codereview.appspot.com/87130045/diff/40001/environs/sync/sync_test.go#newcode275
environs/sync/sync_test.go:275: os.Setenv("GOPATH", gopath)
do we *have* to mess with the real environment? I accept that *perhaps*
we do, but, eww.

https://codereview.appspot.com/87130045/diff/40001/environs/sync/sync_test.go#newcode280
environs/sync/sync_test.go:280: os.Setenv("GOPATH", os.Getenv("GOPATH"))
This is wrong. You need to capture the result of Getenv at setup time --
sadly, you can't just drop the defer and move the code and actually have
it work the same ;).

https://codereview.appspot.com/87130045/diff/40001/environs/sync/sync_test.go#newcode440
environs/sync/sync_test.go:440: // c.Assert(version{}, gc.IsNil)
c.Assert(vers, gc.DeepEquals, version.Binary{}) ?

https://codereview.appspot.com/87130045/diff/40001/environs/tools/build.go
File environs/tools/build.go (right):

https://codereview.appspot.com/87130045/diff/40001/environs/tools/build.go#newcode201
environs/tools/build.go:201: // BundleToolsFunc is a function which can
bundles all the current juju tools
s/bundles/bundle/

https://codereview.appspot.com/87130045/diff/40001/environs/tools/testing/testing.go
File environs/tools/testing/testing.go (right):

https://codereview.appspot.com/87130045/diff/40001/environs/tools/testing/testing.go#newcode34
environs/tools/testing/testing.go:34: // mockUploadTools simulates the
effect of tools.Upload, but skips the time-
GetMockUploadTools returns an UploadFunc that...

https://codereview.appspot.com/87130045/diff/40001/environs/tools/testing/testing.go#newcode37
environs/tools/testing/testing.go:37: // exposed and can itself be
neatly mocked?
I feel like fixing *this* TODO might have been a step in a slightly
better direction, but I'm not totally sure at this point.

edit: I'm getting surer. I'll be around later tonight, let's chat live
-- it's been a while since I saw this code, and I might be missing
something, but I think there's something funky about the layering here.

https://codereview.appspot.com/87130045/diff/40001/environs/tools/testing/testing.go#newcode49
environs/tools/testing/testing.go:49: versions = append(versions,
newVers)
Don't we have a SeriesToUpload func somewhere? Shouldn't we be using
that, rather copy/pasting its functionality? (or, better, mocking a
slightly different layer such that the *only* thing we patch out is the
costly build operation -- ISTM that all the rest of the time we really
just want to patch out the building, because we can cheaply upload to
dummy-provider storage anyway. Right?)

https://codereview.appspot.com/87130045/di...

[Jesse Meek (waigani)]

https://codereview.appspot.com/87130045/diff/40001/environs/bootstrap/bootstrap_test.go
File environs/bootstrap/bootstrap_test.go (right):

https://codereview.appspot.com/87130045/diff/40001/environs/bootstrap/bootstrap_test.go#newcode24
environs/bootstrap/bootstrap_test.go:24: ttesting
"launchpad.net/juju-core/environs/tools/testing"
On 2014/05/07 12:52:05, fwereade wrote:
> explicit "toolstesting" throughout please

Done.

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

https://codereview.appspot.com/87130045/diff/40001/environs/sync/sync_test.go#newcode275
environs/sync/sync_test.go:275: os.Setenv("GOPATH", gopath)
On 2014/05/07 12:52:06, fwereade wrote:
> do we *have* to mess with the real environment? I accept that
*perhaps* we do,
> but, eww.

Done. Using s.PatchEnvPathPrepend.

https://codereview.appspot.com/87130045/diff/40001/environs/sync/sync_test.go#newcode280
environs/sync/sync_test.go:280: os.Setenv("GOPATH", os.Getenv("GOPATH"))
On 2014/05/07 12:52:05, fwereade wrote:
> This is wrong. You need to capture the result of Getenv at setup time
-- sadly,
> you can't just drop the defer and move the code and actually have it
work the
> same ;).

Done. I've mocked go cmd directly.

https://codereview.appspot.com/87130045/diff/40001/environs/sync/sync_test.go#newcode440
environs/sync/sync_test.go:440: // c.Assert(version{}, gc.IsNil)
On 2014/05/07 12:52:06, fwereade wrote:
> c.Assert(vers, gc.DeepEquals, version.Binary{}) ?

Done.

https://codereview.appspot.com/87130045/diff/40001/environs/tools/testing/testing.go
File environs/tools/testing/testing.go (right):

https://codereview.appspot.com/87130045/diff/40001/environs/tools/testing/testing.go#newcode37
environs/tools/testing/testing.go:37: // exposed and can itself be
neatly mocked?
On 2014/05/07 12:52:06, fwereade wrote:
> I feel like fixing *this* TODO might have been a step in a slightly
better
> direction, but I'm not totally sure at this point.

> edit: I'm getting surer. I'll be around later tonight, let's chat live
-- it's
> been a while since I saw this code, and I might be missing something,
but I
> think there's something funky about the layering here.

UploadTools was being mocked in several places in the tests before my
cl. Each place reproduced the same functionality, so I created this
function clean things up.

As such, shouldn't removing UploadTools be done in a separate branch? If
so, for now shouldn't we leave this function as it at least cleans up
all the stray mock UploadTools funcs and eases making UploadTools
unmockable?

What are you referring to when you talk about build logic in
agent/tools?

https://codereview.appspot.com/87130045/diff/40001/environs/tools/testing/testing.go#newcode62
environs/tools/testing/testing.go:62: toolsDir =
filepath.Join(c.MkDir(), "jujud")
On 2014/05/07 12:52:06, fwereade wrote:
> This seems strange to me... AFAICS the only client passes in "", and
the
> replacement "jujud" directory is always empty anyway, so can't we
just: drop the
> dir param, skip the Archive call, write nothing, return the hash of
the empty
> file, and be done?

Done.

https://codereview....

[Jesse Meek (waigani)]

Please take a look.

https://codereview.appspot.com/87130045/

[Jesse Meek (waigani)]

On 2014/05/14 02:49:37, waigani wrote:
> Please take a look.

Please see my comments here:
https://codereview.appspot.com/87130045/#msg7

https://codereview.appspot.com/87130045/

[William Reade (fwereade)]

ISTM that we don't use GetMockUploadTools all *that* much, and it'd be
worth dropping it (and just patching out the building/bundling, so we
get fake tools put in the right place, in the right way). As it is I
think GMUT is a liability, and it needs to be dropped either in this CL
or a followup (but a followup *soon* -- I really don't want it hanging
around).

Please run this by wallyworld again, as well: he's much more au fait
with the various tricky little details around here, but if he's happy I
will be too (modulo my various comments here).

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

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode262
environs/sync/sync_test.go:262: type badBuildSuite struct {
btw, please don't just insert this test suite in between the definition
and the methods of the uploadSuite

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode273
environs/sync/sync_test.go:273: // Create bad Go source file
This isn't accurate any more

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode288
environs/sync/sync_test.go:288: c.Assert(err, gc.IsNil)
blank lines before comments in general, please

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode292
environs/sync/sync_test.go:292: c.Assert(string(out), gc.Equals, "")
I'm not sure you *really* need this bit, but equally you don't have to
take it out :).

The super-sweet spot would be to pull all the MakeTool-style
functionality out into its own helpers and use all that same (tested)
test-support code elsewhere -- but if you want to do that, it'd deserve
its own CL.

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode317
environs/sync/sync_test.go:317: // sync.bundleTools can be mocked to
improve test speed.
It's not 100% clear what this test is meant to be doing, indeed. Given
what we have elsewhere I think I'd be testing this stuff by mocking the
build/bundle stuff, and directly checking what gets uploaded.

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode452
environs/sync/sync_test.go:452: c.Assert(err, gc.ErrorMatches, `build
command "go" failed: exit status 1; `)
please break all these cases into separate test methods where possible

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode480
environs/sync/sync_test.go:480: func (s *uploadSuite)
TestMockBundleTools(c *gc.C) {
The SUT here is BuildToolsTarball, isn't it? We're testing that it calls
BundleTools in the expected way.

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode483
environs/sync/sync_test.go:483: writer io.Writer
writer seems unused -- I'd expect us to write something to it and check
that it was somehow reflected in the result of BuildToolsTarball...

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode493
environs/sync/sync_test.go:493:
sync.BuildToolsTarball(&version.Current.Number)
error/result check?

https://codereview.appspo...

[Jesse Meek (waigani)]

Please take a look.

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

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode262
environs/sync/sync_test.go:262: type badBuildSuite struct {
On 2014/05/15 08:57:20, fwereade wrote:
> btw, please don't just insert this test suite in between the
definition and the
> methods of the uploadSuite

Ah right. The suite goes before the methods for the suite. Got it.

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode273
environs/sync/sync_test.go:273: // Create bad Go source file
On 2014/05/15 08:57:20, fwereade wrote:
> This isn't accurate any more

Done.

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode288
environs/sync/sync_test.go:288: c.Assert(err, gc.IsNil)
On 2014/05/15 08:57:20, fwereade wrote:
> blank lines before comments in general, please

Done.

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode292
environs/sync/sync_test.go:292: c.Assert(string(out), gc.Equals, "")
On 2014/05/15 08:57:19, fwereade wrote:
> I'm not sure you *really* need this bit, but equally you don't have to
take it
> out :).

> The super-sweet spot would be to pull all the MakeTool-style
functionality out
> into its own helpers and use all that same (tested) test-support code
elsewhere
> -- but if you want to do that, it'd deserve its own CL.

Do you mean a helper suite i.e. FakeToolsSuite or a helper package like
utils?

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode317
environs/sync/sync_test.go:317: // sync.bundleTools can be mocked to
improve test speed.
On 2014/05/15 08:57:19, fwereade wrote:
> It's not 100% clear what this test is meant to be doing, indeed. Given
what we
> have elsewhere I think I'd be testing this stuff by mocking the
build/bundle
> stuff, and directly checking what gets uploaded.

Done. Is TestUploadToolsBadBuild enough?

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode452
environs/sync/sync_test.go:452: c.Assert(err, gc.ErrorMatches, `build
command "go" failed: exit status 1; `)
On 2014/05/15 08:57:19, fwereade wrote:
> please break all these cases into separate test methods where possible

Done.

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode480
environs/sync/sync_test.go:480: func (s *uploadSuite)
TestMockBundleTools(c *gc.C) {
On 2014/05/15 08:57:20, fwereade wrote:
> The SUT here is BuildToolsTarball, isn't it? We're testing that it
calls
> BundleTools in the expected way.

I don't follow sorry. I was testing if the mocked BundleTools gets
called. I could have easily used sync.Upload instead. I also don't know
what SUT means sorry.

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode483
environs/sync/sync_test.go:483: writer io.Writer
On 2014/05/15 08:57:19, fwereade wrote:
> writer seems unused -- I'd expect us to write something to it and
check that it
> was somehow reflected in the result of BuildToolsTarball...

Done.

https://code...

[Ian Booth (wallyworld)]

LGTM

https://codereview.appspot.com/87130045/

[William Reade (fwereade)]

Thanks, this is great. LGTM with trivials.

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

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode292
environs/sync/sync_test.go:292: c.Assert(string(out), gc.Equals, "")
On 2014/05/16 03:06:07, waigani wrote:
> On 2014/05/15 08:57:19, fwereade wrote:
> > I'm not sure you *really* need this bit, but equally you don't have
to take it
> > out :).
> >
> > The super-sweet spot would be to pull all the MakeTool-style
functionality out
> > into its own helpers and use all that same (tested) test-support
code
> elsewhere
> > -- but if you want to do that, it'd deserve its own CL.

> Do you mean a helper suite i.e. FakeToolsSuite or a helper package
like utils?

I'd guess a suite would work well for easy integration of
setup/teardown, but not necessarily. Whatever seems to work best :).

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode317
environs/sync/sync_test.go:317: // sync.bundleTools can be mocked to
improve test speed.
On 2014/05/16 03:06:08, waigani wrote:
> On 2014/05/15 08:57:19, fwereade wrote:
> > It's not 100% clear what this test is meant to be doing, indeed.
Given what we
> > have elsewhere I think I'd be testing this stuff by mocking the
build/bundle
> > stuff, and directly checking what gets uploaded.

> Done. Is TestUploadToolsBadBuild enough?

Yeah, as long as we exercise both the happy path and the sad one, I
think we're good.

https://codereview.appspot.com/87130045/diff/80001/environs/sync/sync_test.go#newcode480
environs/sync/sync_test.go:480: func (s *uploadSuite)
TestMockBundleTools(c *gc.C) {
On 2014/05/16 03:06:08, waigani wrote:
> On 2014/05/15 08:57:20, fwereade wrote:
> > The SUT here is BuildToolsTarball, isn't it? We're testing that it
calls
> > BundleTools in the expected way.

> I don't follow sorry. I was testing if the mocked BundleTools gets
called. I
> could have easily used sync.Upload instead. I also don't know what SUT
means
> sorry.

System Under Test. And no need to apologise :).

https://codereview.appspot.com/87130045/diff/80001/environs/tools/testing/testing.go
File environs/tools/testing/testing.go (right):

https://codereview.appspot.com/87130045/diff/80001/environs/tools/testing/testing.go#newcode52
environs/tools/testing/testing.go:52: }
On 2014/05/16 03:06:08, waigani wrote:
> On 2014/05/15 08:57:20, fwereade wrote:
> > I'm still nervous that this is a parallel implementation of logic in
> uploadTools
> > itself that *will* invitably change out of sync with this
implementation. This
> > is basically how mocking ends up going wrong IMO...

> Done. It's GONE GONE GONE!

w00t!

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

https://codereview.appspot.com/87130045/diff/100001/environs/sync/sync_test.go#newcode284
environs/sync/sync_test.go:284: // sync.bundleTools can be mocked to
improve test speed.
Hey, I think it's clicked. It's downloading the tools (and doing some
basic checks on them) to check that they really got uploaded. I think
I'd be h...

[William Reade (fwereade)]

On 2014/05/16 08:29:36, fwereade wrote:
> Thanks, this is great. LGTM with trivials.

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

https://codereview.appspot.com/87130045/diff/100001/environs/sync/sync_test.go#newcode284
> environs/sync/sync_test.go:284: // sync.bundleTools can be mocked to
improve
> test speed.
> Hey, I think it's clicked. It's downloading the tools (and doing some
basic
> checks on them) to check that they really got uploaded. I think I'd be
happiest
> to see more direct, explicit tests: I think we actually have pretty
detailed
> ones around bootstrap/upgrade-juju, but it'd be best to have more
detail in the
> Upload tests and maybe a bit less in the cmd/juju ones.

I don't want that in this CL, though, it's big enough already.

https://codereview.appspot.com/87130045/

[Go Bot (go-bot)]

The attempt to merge lp:~waigani/juju-core/1307241-isolate-from-jujud into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.014s
ok launchpad.net/juju-core/agent 0.852s
ok launchpad.net/juju-core/agent/mongo 0.439s
ok launchpad.net/juju-core/agent/tools 0.194s
ok launchpad.net/juju-core/bzr 5.411s
ok launchpad.net/juju-core/cert 2.902s
ok launchpad.net/juju-core/charm 0.459s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.026s
ok launchpad.net/juju-core/cloudinit/sshinit 0.730s
ok launchpad.net/juju-core/cmd 0.163s
ok launchpad.net/juju-core/cmd/charm-admin 0.260s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.209s
ok launchpad.net/juju-core/cmd/juju 165.358s
ok launchpad.net/juju-core/cmd/jujud 68.830s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 10.541s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.139s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.025s
ok launchpad.net/juju-core/container 0.035s
ok launchpad.net/juju-core/container/factory 0.037s
ok launchpad.net/juju-core/container/kvm 0.196s
ok launchpad.net/juju-core/container/kvm/mock 0.035s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.287s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.234s
ok launchpad.net/juju-core/environs 2.300s

----------------------------------------------------------------------
FAIL: bootstrap_test.go:294: bootstrapSuite.TestEnsureToolsAvailability

[LOG] 0:00.016 INFO juju.utils hostname SSL verification enabled
[LOG] 0:00.016 DEBUG juju.environs.tools reading v1.* tools
[LOG] 0:00.017 INFO juju.environs.testing uploading FAKE tools 1.19.0-precise-amd64
[LOG] 0:00.017 DEBUG juju.environs.httpstorage putting "tools/releases/juju-1.19.0-precise-amd64.tgz" (len 130) to storage
[LOG] 0:00.018 INFO juju.environs.testing uploading FAKE tools 1.19.0-trusty-amd64
[LOG] 0:00.019 DEBUG juju.environs.httpstorage putting "tools/releases/juju-1.19.0-trusty-amd64.tgz" (len 128) to storage
[LOG] 0:00.020 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 0:00.020 DEBUG juju.environs.tools no series specified when finding tools, looking for any
[LOG] 0:00.020 DEBUG juju.environs.httpstorage getting "tools/streams/v1/index.sjson" from storage
[LOG] 0:00.020 DEBUG juju.environs.simplestreams fetchData failed for "http://127.0.0.1:41583/tools/streams/v1/index.sjson": file "tools/streams/v1/index.sjson" not found
[LOG] 0:00.020 DEBUG juju.environs.simplestreams cannot load index "http://127.0.0.1:41583/tools/streams/v1/index.sjson": invalid ...

[Jesse Meek (waigani)]

Please take a look.

https://codereview.appspot.com/87130045/

[Go Bot (go-bot)]

The attempt to merge lp:~waigani/juju-core/1307241-isolate-from-jujud into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.014s
ok launchpad.net/juju-core/agent 0.874s
ok launchpad.net/juju-core/agent/mongo 0.423s
ok launchpad.net/juju-core/agent/tools 0.217s
ok launchpad.net/juju-core/bzr 5.120s
ok launchpad.net/juju-core/cert 2.111s
ok launchpad.net/juju-core/charm 0.461s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.029s
ok launchpad.net/juju-core/cloudinit/sshinit 0.750s
ok launchpad.net/juju-core/cmd 0.174s
ok launchpad.net/juju-core/cmd/charm-admin 0.281s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.201s
ok launchpad.net/juju-core/cmd/juju 159.798s
ok launchpad.net/juju-core/cmd/jujud 67.111s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 7.384s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.162s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.026s
ok launchpad.net/juju-core/container 0.035s
ok launchpad.net/juju-core/container/factory 0.036s
ok launchpad.net/juju-core/container/kvm 0.178s
ok launchpad.net/juju-core/container/kvm/mock 0.035s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.278s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.213s
ok launchpad.net/juju-core/environs 2.314s
ok launchpad.net/juju-core/environs/bootstrap 3.925s
ok launchpad.net/juju-core/environs/cloudinit 0.432s
ok launchpad.net/juju-core/environs/config 1.778s
ok launchpad.net/juju-core/environs/configstore 0.030s
ok launchpad.net/juju-core/environs/filestorage 0.029s
ok launchpad.net/juju-core/environs/httpstorage 0.768s
ok launchpad.net/juju-core/environs/imagemetadata 0.453s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.043s
ok launchpad.net/juju-core/environs/jujutest 0.207s
ok launchpad.net/juju-core/environs/manual 12.169s
? launchpad.net/juju-core/environs/network [no test files]
ok launchpad.net/juju-core/environs/simplestreams 0.288s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.019s
ok launchpad.net/juju-core/environs/storage 0.975s
FAIL launchpad.net/juju-core/environs/sync [build failed]
ok launchpad.net/juju-core/environs/testing 0.197s
ok launchpad.net/juju-core/environs/tools 4.891s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/instance 0.020s
? launchpad.net/juju-core/instance/testing [no test files]
ok launc...

[Go Bot (go-bot)]

The attempt to merge lp:~waigani/juju-core/1307241-isolate-from-jujud into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.012s
ok launchpad.net/juju-core/agent 0.944s
ok launchpad.net/juju-core/agent/mongo 0.478s
ok launchpad.net/juju-core/agent/tools 0.151s
ok launchpad.net/juju-core/bzr 5.002s
ok launchpad.net/juju-core/cert 2.248s
ok launchpad.net/juju-core/charm 0.436s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.037s
ok launchpad.net/juju-core/cloudinit/sshinit 0.833s
ok launchpad.net/juju-core/cmd 0.167s
ok launchpad.net/juju-core/cmd/charm-admin 0.266s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.160s
ok launchpad.net/juju-core/cmd/juju 165.833s
ok launchpad.net/juju-core/cmd/jujud 66.747s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 6.983s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.156s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.025s
ok launchpad.net/juju-core/container 0.035s
ok launchpad.net/juju-core/container/factory 0.036s
ok launchpad.net/juju-core/container/kvm 0.197s
ok launchpad.net/juju-core/container/kvm/mock 0.035s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.246s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.220s
ok launchpad.net/juju-core/environs 2.271s
ok launchpad.net/juju-core/environs/bootstrap 3.844s
ok launchpad.net/juju-core/environs/cloudinit 0.521s
ok launchpad.net/juju-core/environs/config 2.707s
ok launchpad.net/juju-core/environs/configstore 0.029s
ok launchpad.net/juju-core/environs/filestorage 0.024s
ok launchpad.net/juju-core/environs/httpstorage 0.756s
ok launchpad.net/juju-core/environs/imagemetadata 0.472s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.045s
ok launchpad.net/juju-core/environs/jujutest 0.212s
ok launchpad.net/juju-core/environs/manual 9.438s
? launchpad.net/juju-core/environs/network [no test files]
ok launchpad.net/juju-core/environs/simplestreams 0.238s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 0.887s
ok launchpad.net/juju-core/environs/storage 0.861s
FAIL launchpad.net/juju-core/environs/sync [build failed]
ok launchpad.net/juju-core/environs/testing 0.217s
ok launchpad.net/juju-core/environs/tools 4.582s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/instance 0.028s
? launchpad.net/juju-core/instance/testing [no test files]
ok launch...

[Go Bot (go-bot)]

The attempt to merge lp:~waigani/juju-core/1307241-isolate-from-jujud into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.024s
ok launchpad.net/juju-core/agent 0.932s
ok launchpad.net/juju-core/agent/mongo 0.482s
ok launchpad.net/juju-core/agent/tools 0.197s
ok launchpad.net/juju-core/bzr 4.900s
ok launchpad.net/juju-core/cert 3.017s
ok launchpad.net/juju-core/charm 0.459s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.029s
ok launchpad.net/juju-core/cloudinit/sshinit 0.733s
ok launchpad.net/juju-core/cmd 0.170s
ok launchpad.net/juju-core/cmd/charm-admin 0.274s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.251s
ok launchpad.net/juju-core/cmd/juju 160.913s
ok launchpad.net/juju-core/cmd/jujud 67.858s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 8.151s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.147s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.025s
ok launchpad.net/juju-core/container 0.035s
ok launchpad.net/juju-core/container/factory 0.037s
ok launchpad.net/juju-core/container/kvm 0.157s
ok launchpad.net/juju-core/container/kvm/mock 0.040s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.295s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.235s
ok launchpad.net/juju-core/environs 2.264s
ok launchpad.net/juju-core/environs/bootstrap 4.033s
ok launchpad.net/juju-core/environs/cloudinit 0.439s
ok launchpad.net/juju-core/environs/config 1.943s
ok launchpad.net/juju-core/environs/configstore 0.038s
ok launchpad.net/juju-core/environs/filestorage 0.029s
ok launchpad.net/juju-core/environs/httpstorage 0.693s
ok launchpad.net/juju-core/environs/imagemetadata 0.466s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.051s
ok launchpad.net/juju-core/environs/jujutest 0.244s
ok launchpad.net/juju-core/environs/manual 12.372s
? launchpad.net/juju-core/environs/network [no test files]
ok launchpad.net/juju-core/environs/simplestreams 0.312s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 0.907s
ok launchpad.net/juju-core/environs/storage 0.787s

----------------------------------------------------------------------
PANIC: sync_test.go:420: badBuildSuite.SetUpTest

... Panic: cannot share a state between two dummy environs; old "only"; new "only" (PC=0x414311)

/usr/lib/go/src/pkg/runtime/panic.c:229
  in panic
/home/tarmac/trees/src/launchpad.net/juju-core/provider/dummy/environs.go:48...

[Go Bot (go-bot)]

Attempt to merge into lp:juju-core failed due to conflicts:

text conflict in environs/jujutest/livetests.go

[John A Meinel (jameinel)]

I'm not sure what changed, but Tarmac really doesn't like this branch:
014-05-20 10:12:04 ERROR An error occurred trying to merge lp:juju-core: Server sent an unexpected error: ('error', 'GhostRevisionsHaveNoRevno', 'Could not determine revno for {<email address hidden>} because its ancestry shows a ghost at {tarmac-20140413170514-ii8w6q2fht3eiizi}')
2014-05-20 10:12:04 ERROR Traceback (most recent call last):
  File "/home/tarmac/tarmac/tarmac/bin/commands.py", line 385, in run
    merged = self._do_merges(branch)
  File "/home/tarmac/tarmac/tarmac/bin/commands.py", line 227, in _do_merges
    str(proposal.reviewed_revid))
  File "<string>", line 4, in revision_id_to_revno_read_locked
  File "/home/tarmac/.local/lib/python2.7/site-packages/bzrlib/remote.py", line 3885, in revision_id_to_revno
    self._remote_path(), revision_id)
  File "/home/tarmac/.local/lib/python2.7/site-packages/bzrlib/remote.py", line 75, in _call
    self._translate_error(err, **err_context)
  File "/home/tarmac/.local/lib/python2.7/site-packages/bzrlib/remote.py", line 3454, in _translate_error
    self.repository._translate_error(err, branch=self, **context)
  File "/home/tarmac/.local/lib/python2.7/site-packages/bzrlib/remote.py", line 1347, in _translate_error
    self.bzrdir._translate_error(err, repository=self, **context)
  File "/home/tarmac/.local/lib/python2.7/site-packages/bzrlib/remote.py", line 491, in _translate_error
    _translate_error(err, bzrdir=self, **context)
  File "/home/tarmac/.local/lib/python2.7/site-packages/bzrlib/remote.py", line 4194, in _translate_error
    raise errors.UnknownErrorFromSmartServer(err)
UnknownErrorFromSmartServer: Server sent an unexpected error: ('error', 'GhostRevisionsHaveNoRevno', 'Could not determine revno for {<email address hidden>} because its ancestry shows a ghost at {tarmac-20140413170514-ii8w6q2fht3eiizi}')

=== modified file 'cmd/juju/bootstrap_test.go'
--- cmd/juju/bootstrap_test.go	2014-05-16 01:33:13 +0000
+++ cmd/juju/bootstrap_test.go	2014-05-20 00:26:58 +0000
@@ -27,7 +27,7 @@
 	"launchpad.net/juju-core/environs/sync"
 	envtesting "launchpad.net/juju-core/environs/testing"
 	envtools "launchpad.net/juju-core/environs/tools"
-	ttesting "launchpad.net/juju-core/environs/tools/testing"
+	toolstesting "launchpad.net/juju-core/environs/tools/testing"
 	"launchpad.net/juju-core/juju/arch"
 	"launchpad.net/juju-core/provider/dummy"
 	coretesting "launchpad.net/juju-core/testing"
@@ -56,6 +56,8 @@
 	// Set up a local source with tools.
 	sourceDir := createToolsSource(c, vAll)
 	s.PatchValue(&envtools.DefaultBaseURL, sourceDir)
+
+	s.PatchValue(&envtools.BundleTools, toolstesting.GetMockBundleTools(c))
 }
 
 func (s *BootstrapSuite) TearDownSuite(c *gc.C) {
@@ -149,32 +151,7 @@
 	c.Check(stripped, gc.Matches, test.err)
 }
 
-// mockUploadTools simulates the effect of tools.Upload, but skips the time-
-// consuming build from source.
-// TODO(fwereade) better factor agent/tools such that build logic is
-// exposed and can itself be neatly mocked?
-func mockUploadTools(stor storage.Storage, forceVersion *version.Number, series ...string) (*coretools.Tools, error) {
-	vers := version.Current
-	if forceVersion != nil {
-		vers.Number = *forceVersion
-	}
-	versions := []version.Binary{vers}
-	for _, series := range series {
-		if series != version.Current.Series {
-			newVers := vers
-			newVers.Series = series
-			versions = append(versions, newVers)
-		}
-	}
-	agentTools, err := envtesting.UploadFakeToolsVersions(stor, versions...)
-	if err != nil {
-		return nil, err
-	}
-	return agentTools[0], nil
-}
-
 func (s *BootstrapSuite) TestTest(c *gc.C) {
-	s.PatchValue(&sync.Upload, mockUploadTools)
 	for i, test := range bootstrapTests {
 		c.Logf("\ntest %d: %s", i, test.info)
 		test.run(c)
@@ -542,7 +519,7 @@
 }
 
 func (s *BootstrapSuite) setupAutoUploadTest(c *gc.C, vers, series string) environs.Environ {
-	s.PatchValue(&sync.Upload, mockUploadTools)
+	s.PatchValue(&envtools.BundleTools, toolstesting.GetMockBundleTools(c))
 	sourceDir := createToolsSource(c, vAll)
 	s.PatchValue(&envtools.DefaultBaseURL, sourceDir)
 
@@ -659,7 +636,7 @@
 		versionStrings[i] = vers.String()
 	}
 	source := c.MkDir()
-	ttesting.MakeTools(c, source, "releases", versionStrings)
+	toolstesting.MakeTools(c, source, "releases", versionStrings)
 	return source
 }
 

=== modified file 'cmd/juju/upgradejuju_test.go'
--- cmd/juju/upgradejuju_test.go	2014-05-12 04:28:39 +0000
+++ cmd/juju/upgradejuju_test.go	2014-05-20 00:26:58 +0000
@@ -21,6 +21,7 @@
 	"launchpad.net/juju-core/environs/sync"
 	envtesting "launchpad.net/juju-core/environs/testing"
 	envtools "launchpad.net/juju-core/environs/tools"
+	toolstesting "launchpad.net/juju-core/environs/tools/testing"
 	"launchpad.net/juju-core/juju/testing"
 	coretesting "launchpad.net/juju-core/testing"
 	"launchpad.net/juju-core/version"
@@ -275,33 +276,7 @@
 	expectUploaded: []string{"2.7.3.2-quantal-amd64", "2.7.3.2-%LTS%-amd64", "2.7.3.2-raring-amd64"},
 }}
 
-// getMockBuildTools returns a sync.BuildToolsTarballFunc implementation which generates
-// a fake tools tarball.
-func (s *UpgradeJujuSuite) getMockBuildTools(c *gc.C) sync.BuildToolsTarballFunc {
-	return func(forceVersion *version.Number) (*sync.BuiltTools, error) {
-		// UploadFakeToolsVersions requires a storage to write to.
-		stor, err := filestorage.NewFileStorageWriter(s.toolsDir)
-		c.Assert(err, gc.IsNil)
-		vers := version.Current
-		if forceVersion != nil {
-			vers.Number = *forceVersion
-		}
-		versions := []version.Binary{vers}
-		uploadedTools, err := envtesting.UploadFakeToolsVersions(stor, versions...)
-		c.Assert(err, gc.IsNil)
-		agentTools := uploadedTools[0]
-		return &sync.BuiltTools{
-			Dir:         s.toolsDir,
-			StorageName: envtools.StorageName(vers),
-			Version:     vers,
-			Size:        agentTools.Size,
-			Sha256Hash:  agentTools.SHA256,
-		}, nil
-	}
-}
-
 func (s *UpgradeJujuSuite) TestUpgradeJuju(c *gc.C) {
-	s.PatchValue(&sync.BuildToolsTarball, s.getMockBuildTools(c))
 	oldVersion := version.Current
 	defer func() {
 		version.Current = oldVersion
@@ -415,7 +390,7 @@
 	}
 	err := s.State.UpdateEnvironConfig(updateAttrs, nil, nil)
 	c.Assert(err, gc.IsNil)
-	s.toolsDir = c.MkDir()
+	s.PatchValue(&sync.BuildToolsTarball, toolstesting.GetMockBuildTools(c))
 }
 
 func (s *UpgradeJujuSuite) TestUpgradeJujuWithRealUpload(c *gc.C) {

=== modified file 'cmd/plugins/juju-metadata/toolsmetadata_test.go'
--- cmd/plugins/juju-metadata/toolsmetadata_test.go	2014-05-16 01:33:13 +0000
+++ cmd/plugins/juju-metadata/toolsmetadata_test.go	2014-05-20 00:26:58 +0000
@@ -19,7 +19,7 @@
 	"launchpad.net/juju-core/environs/configstore"
 	envtesting "launchpad.net/juju-core/environs/testing"
 	"launchpad.net/juju-core/environs/tools"
-	ttesting "launchpad.net/juju-core/environs/tools/testing"
+	toolstesting "launchpad.net/juju-core/environs/tools/testing"
 	"launchpad.net/juju-core/juju/osenv"
 	"launchpad.net/juju-core/provider/dummy"
 	coretesting "launchpad.net/juju-core/testing"
@@ -95,13 +95,13 @@
 
 func (s *ToolsMetadataSuite) TestGenerateDefaultDirectory(c *gc.C) {
 	metadataDir := osenv.JujuHome() // default metadata dir
-	ttesting.MakeTools(c, metadataDir, "releases", versionStrings)
+	toolstesting.MakeTools(c, metadataDir, "releases", versionStrings)
 	ctx := coretesting.Context(c)
 	code := cmd.Main(envcmd.Wrap(&ToolsMetadataCommand{}), ctx, nil)
 	c.Assert(code, gc.Equals, 0)
 	output := ctx.Stdout.(*bytes.Buffer).String()
 	c.Assert(output, gc.Matches, expectedOutputDirectory)
-	metadata := ttesting.ParseMetadataFromDir(c, metadataDir, false)
+	metadata := toolstesting.ParseMetadataFromDir(c, metadataDir, false)
 	c.Assert(metadata, gc.HasLen, len(versionStrings))
 	obtainedVersionStrings := make([]string, len(versionStrings))
 	for i, metadata := range metadata {
@@ -113,13 +113,13 @@
 
 func (s *ToolsMetadataSuite) TestGenerateDirectory(c *gc.C) {
 	metadataDir := c.MkDir()
-	ttesting.MakeTools(c, metadataDir, "releases", versionStrings)
+	toolstesting.MakeTools(c, metadataDir, "releases", versionStrings)
 	ctx := coretesting.Context(c)
 	code := cmd.Main(envcmd.Wrap(&ToolsMetadataCommand{}), ctx, []string{"-d", metadataDir})
 	c.Assert(code, gc.Equals, 0)
 	output := ctx.Stdout.(*bytes.Buffer).String()
 	c.Assert(output, gc.Matches, expectedOutputDirectory)
-	metadata := ttesting.ParseMetadataFromDir(c, metadataDir, false)
+	metadata := toolstesting.ParseMetadataFromDir(c, metadataDir, false)
 	c.Assert(metadata, gc.HasLen, len(versionStrings))
 	obtainedVersionStrings := make([]string, len(versionStrings))
 	for i, metadata := range metadata {
@@ -131,14 +131,14 @@
 
 func (s *ToolsMetadataSuite) TestGenerateWithPublicFallback(c *gc.C) {
 	// Write tools and metadata to the public tools location.
-	ttesting.MakeToolsWithCheckSum(c, s.publicStorageDir, "releases", versionStrings)
+	toolstesting.MakeToolsWithCheckSum(c, s.publicStorageDir, "releases", versionStrings)
 
 	// Run the command with no local metadata.
 	ctx := coretesting.Context(c)
 	metadataDir := c.MkDir()
 	code := cmd.Main(envcmd.Wrap(&ToolsMetadataCommand{}), ctx, []string{"-d", metadataDir})
 	c.Assert(code, gc.Equals, 0)
-	metadata := ttesting.ParseMetadataFromDir(c, metadataDir, false)
+	metadata := toolstesting.ParseMetadataFromDir(c, metadataDir, false)
 	c.Assert(metadata, gc.HasLen, len(versionStrings))
 	obtainedVersionStrings := make([]string, len(versionStrings))
 	for i, metadata := range metadata {
@@ -150,13 +150,13 @@
 
 func (s *ToolsMetadataSuite) TestGenerateWithMirrors(c *gc.C) {
 	metadataDir := c.MkDir()
-	ttesting.MakeTools(c, metadataDir, "releases", versionStrings)
+	toolstesting.MakeTools(c, metadataDir, "releases", versionStrings)
 	ctx := coretesting.Context(c)
 	code := cmd.Main(envcmd.Wrap(&ToolsMetadataCommand{}), ctx, []string{"--public", "-d", metadataDir})
 	c.Assert(code, gc.Equals, 0)
 	output := ctx.Stdout.(*bytes.Buffer).String()
 	c.Assert(output, gc.Matches, expectedOutputMirrors)
-	metadata := ttesting.ParseMetadataFromDir(c, metadataDir, true)
+	metadata := toolstesting.ParseMetadataFromDir(c, metadataDir, true)
 	c.Assert(metadata, gc.HasLen, len(versionStrings))
 	obtainedVersionStrings := make([]string, len(versionStrings))
 	for i, metadata := range metadata {
@@ -184,7 +184,7 @@
 		currentVersion.String() + ".1-precise-amd64",
 	}
 	metadataDir := osenv.JujuHome() // default metadata dir
-	ttesting.MakeTools(c, metadataDir, "releases", versionStrings)
+	toolstesting.MakeTools(c, metadataDir, "releases", versionStrings)
 	ctx := coretesting.Context(c)
 	code := cmd.Main(envcmd.Wrap(&ToolsMetadataCommand{}), ctx, nil)
 	c.Assert(code, gc.Equals, 0)
@@ -197,11 +197,11 @@
 .*Writing tools/streams/v1/com\.ubuntu\.juju:released:tools\.json
 `[1:], regexp.QuoteMeta(versionStrings[0]), regexp.QuoteMeta(versionStrings[1]))
 	c.Assert(output, gc.Matches, expectedOutput)
-	metadata := ttesting.ParseMetadataFromDir(c, metadataDir, false)
+	metadata := toolstesting.ParseMetadataFromDir(c, metadataDir, false)
 	c.Assert(metadata, gc.HasLen, 2)
 
 	filename := fmt.Sprintf("juju-%s-precise-amd64.tgz", currentVersion)
-	size, sha256 := ttesting.SHA256sum(c, filepath.Join(metadataDir, "tools", "releases", filename))
+	size, sha256 := toolstesting.SHA256sum(c, filepath.Join(metadataDir, "tools", "releases", filename))
 	c.Assert(metadata[0], gc.DeepEquals, &tools.ToolsMetadata{
 		Release:  "precise",
 		Version:  currentVersion.String(),
@@ -213,7 +213,7 @@
 	})
 
 	filename = fmt.Sprintf("juju-%s.1-precise-amd64.tgz", currentVersion)
-	size, sha256 = ttesting.SHA256sum(c, filepath.Join(metadataDir, "tools", "releases", filename))
+	size, sha256 = toolstesting.SHA256sum(c, filepath.Join(metadataDir, "tools", "releases", filename))
 	c.Assert(metadata[1], gc.DeepEquals, &tools.ToolsMetadata{
 		Release:  "precise",
 		Version:  currentVersion.String() + ".1",

=== modified file 'environs/bootstrap/bootstrap_test.go'
--- environs/bootstrap/bootstrap_test.go	2014-05-15 05:14:01 +0000
+++ environs/bootstrap/bootstrap_test.go	2014-05-20 00:26:58 +0000
@@ -16,12 +16,11 @@
 	"launchpad.net/juju-core/environs/bootstrap"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/environs/configstore"
-	"launchpad.net/juju-core/environs/filestorage"
 	"launchpad.net/juju-core/environs/simplestreams"
 	"launchpad.net/juju-core/environs/storage"
-	"launchpad.net/juju-core/environs/sync"
 	envtesting "launchpad.net/juju-core/environs/testing"
 	envtools "launchpad.net/juju-core/environs/tools"
+	toolstesting "launchpad.net/juju-core/environs/tools/testing"
 	"launchpad.net/juju-core/juju/arch"
 	"launchpad.net/juju-core/provider/dummy"
 	coretesting "launchpad.net/juju-core/testing"
@@ -156,6 +155,7 @@
 	}}
 
 func (s *bootstrapSuite) TestBootstrapTools(c *gc.C) {
+	s.PatchValue(&envtools.BundleTools, toolstesting.GetMockBundleTools(c))
 	allTests := append(envtesting.BootstrapToolsTests, bootstrapSetAgentVersionTests...)
 	// version.Current is set in the loop so ensure it is restored later.
 	s.PatchValue(&version.Current, version.Current)
@@ -290,32 +290,6 @@
 		"cannot upload bootstrap tools: Juju cannot bootstrap because no tools are available for your environment.*")
 }
 
-// getMockBuildTools returns a sync.BuildToolsTarballFunc implementation which generates
-// a fake tools tarball.
-func (s *bootstrapSuite) getMockBuildTools(c *gc.C) sync.BuildToolsTarballFunc {
-	toolsDir := c.MkDir()
-	return func(forceVersion *version.Number) (*sync.BuiltTools, error) {
-		// UploadFakeToolsVersions requires a storage to write to.
-		stor, err := filestorage.NewFileStorageWriter(toolsDir)
-		c.Assert(err, gc.IsNil)
-		vers := version.Current
-		if forceVersion != nil {
-			vers.Number = *forceVersion
-		}
-		versions := []version.Binary{vers}
-		uploadedTools, err := envtesting.UploadFakeToolsVersions(stor, versions...)
-		c.Assert(err, gc.IsNil)
-		agentTools := uploadedTools[0]
-		return &sync.BuiltTools{
-			Dir:         toolsDir,
-			StorageName: envtools.StorageName(vers),
-			Version:     vers,
-			Size:        agentTools.Size,
-			Sha256Hash:  agentTools.SHA256,
-		}, nil
-	}
-}
-
 func (s *bootstrapSuite) TestEnsureToolsAvailability(c *gc.C) {
 	existingToolsVersion := version.MustParseBinary("1.19.0-trusty-amd64")
 	s.PatchValue(&version.Current, existingToolsVersion)
@@ -326,7 +300,7 @@
 	cliVersion := version.Current
 	cliVersion.Arch = "arm64"
 	version.Current = cliVersion
-	s.PatchValue(&sync.BuildToolsTarball, s.getMockBuildTools(c))
+	s.PatchValue(&envtools.BundleTools, toolstesting.GetMockBundleTools(c))
 	// Host runs arm64, environment supports arm64.
 	s.PatchValue(&arch.HostArch, func() string {
 		return "arm64"
@@ -378,7 +352,7 @@
 	cliVersion := version.Current
 	cliVersion.Arch = "arm64"
 	version.Current = cliVersion
-	s.PatchValue(&sync.BuildToolsTarball, s.getMockBuildTools(c))
+	s.PatchValue(&envtools.BundleTools, toolstesting.GetMockBundleTools(c))
 	// Host runs arm64, environment supports arm64.
 	s.PatchValue(&arch.HostArch, func() string {
 		return "arm64"

=== modified file 'environs/sync/sync_test.go'
--- environs/sync/sync_test.go	2014-05-16 01:33:13 +0000
+++ environs/sync/sync_test.go	2014-05-20 00:26:58 +0000
@@ -12,7 +12,6 @@
 	"os/exec"
 	"path/filepath"
 	"sort"
-	"strings"
 	"testing"
 
 	jc "github.com/juju/testing/checkers"
@@ -27,9 +26,10 @@
 	"launchpad.net/juju-core/environs/sync"
 	envtesting "launchpad.net/juju-core/environs/testing"
 	envtools "launchpad.net/juju-core/environs/tools"
-	ttesting "launchpad.net/juju-core/environs/tools/testing"
+	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"
@@ -50,6 +50,7 @@
 
 var _ = gc.Suite(&syncSuite{})
 var _ = gc.Suite(&uploadSuite{})
+var _ = gc.Suite(&badBuildSuite{})
 
 func (s *syncSuite) setUpTest(c *gc.C) {
 	s.FakeJujuHomeSuite.SetUpTest(c)
@@ -86,8 +87,8 @@
 	for i, vers := range vAll {
 		versionStrings[i] = vers.String()
 	}
-	ttesting.MakeTools(c, baseDir, "releases", versionStrings)
-	ttesting.MakeTools(c, s.localStorage, "releases", versionStrings)
+	toolstesting.MakeTools(c, baseDir, "releases", versionStrings)
+	toolstesting.MakeTools(c, s.localStorage, "releases", versionStrings)
 
 	// Switch the default tools location.
 	baseURL, err := s.storage.URL(storage.BaseToolsPath)
@@ -279,6 +280,8 @@
 	c.Assert(err, gc.IsNil)
 	c.Assert(t.Version, gc.Equals, version.Current)
 	c.Assert(t.URL, gc.Not(gc.Equals), "")
+	// TODO(waigani) Does this test need to download tools? If not,
+	// sync.bundleTools can be mocked to improve test speed.
 	dir := downloadTools(c, t)
 	out, err := exec.Command(filepath.Join(dir, "jujud"), "version").CombinedOutput()
 	c.Assert(err, gc.IsNil)
@@ -307,38 +310,14 @@
 	c.Assert(t.Version, gc.Equals, vers)
 }
 
-// Test that the upload procedure fails correctly
-// when the build process fails (because of a bad Go source
-// file in this case).
-func (s *uploadSuite) TestUploadBadBuild(c *gc.C) {
-	gopath := c.MkDir()
-	join := append([]string{gopath, "src"}, strings.Split("launchpad.net/juju-core/cmd/broken", "/")...)
-	pkgdir := filepath.Join(join...)
-	err := os.MkdirAll(pkgdir, 0777)
-	c.Assert(err, gc.IsNil)
-
-	err = ioutil.WriteFile(filepath.Join(pkgdir, "broken.go"), []byte("nope"), 0666)
-	c.Assert(err, gc.IsNil)
-
-	defer os.Setenv("GOPATH", os.Getenv("GOPATH"))
-	os.Setenv("GOPATH", gopath)
-
-	t, err := sync.Upload(s.env.Storage(), nil)
-	c.Assert(t, gc.IsNil)
-	c.Assert(err, gc.ErrorMatches, `build command "go" failed: exit status 1; can't load package:(.|\n)*`)
-}
-
 func (s *uploadSuite) TestSyncTools(c *gc.C) {
+	s.PatchValue(&envtools.BundleTools, toolstesting.GetMockBundleTools(c))
 	builtTools, err := sync.BuildToolsTarball(nil)
 	c.Assert(err, gc.IsNil)
 	t, err := sync.SyncBuiltTools(s.env.Storage(), builtTools)
 	c.Assert(err, gc.IsNil)
 	c.Assert(t.Version, gc.Equals, version.Current)
 	c.Assert(t.URL, gc.Not(gc.Equals), "")
-	dir := downloadTools(c, t)
-	out, err := exec.Command(filepath.Join(dir, "jujud"), "version").CombinedOutput()
-	c.Assert(err, gc.IsNil)
-	c.Assert(string(out), gc.Equals, version.Current.String()+"\n")
 }
 
 func (s *uploadSuite) TestSyncToolsFakeSeries(c *gc.C) {
@@ -384,7 +363,7 @@
 		actualRaw := downloadToolsRaw(c, t)
 		c.Assert(string(actualRaw), gc.Equals, string(expectRaw))
 	}
-	metadata := ttesting.ParseMetadataFromStorage(c, s.env.Storage(), false)
+	metadata := toolstesting.ParseMetadataFromStorage(c, s.env.Storage(), false)
 	c.Assert(metadata, gc.HasLen, 3)
 	for i, tm := range metadata {
 		c.Assert(tm.Release, gc.Equals, expectSeries[i])
@@ -417,3 +396,149 @@
 	c.Assert(err, gc.IsNil)
 	return buf.Bytes()
 }
+
+func bundleTools(c *gc.C) (version.Binary, string, error) {
+	f, err := ioutil.TempFile("", "juju-tgz")
+	c.Assert(err, gc.IsNil)
+	defer f.Close()
+	defer os.Remove(f.Name())
+
+	return envtools.BundleTools(f, &version.Current.Number)
+}
+
+type badBuildSuite struct {
+	env environs.Environ
+	testbase.LoggingSuite
+	envtesting.ToolsFixture
+}
+
+var badGo = `
+#!/bin/bash --norc
+exit 1
+`[1:]
+
+func (s *badBuildSuite) SetUpTest(c *gc.C) {
+	s.LoggingSuite.SetUpTest(c)
+	s.ToolsFixture.SetUpTest(c)
+	// We only want to use simplestreams to find any synced tools.
+	cfg, err := config.New(config.NoDefaults, dummy.SampleConfig())
+	c.Assert(err, gc.IsNil)
+	s.env, err = environs.Prepare(cfg, coretesting.Context(c), configstore.NewMem())
+	c.Assert(err, gc.IsNil)
+
+	// Mock go cmd
+	testPath := c.MkDir()
+	s.PatchEnvPathPrepend(testPath)
+	path := filepath.Join(testPath, "go")
+	err = ioutil.WriteFile(path, []byte(badGo), 0755)
+	c.Assert(err, gc.IsNil)
+
+	// Check mocked go cmd errors
+	out, err := exec.Command("go").CombinedOutput()
+	c.Assert(err, gc.ErrorMatches, "exit status 1")
+	c.Assert(string(out), gc.Equals, "")
+}
+
+func (s *badBuildSuite) TearDownTest(c *gc.C) {
+	dummy.Reset()
+	s.ToolsFixture.TearDownTest(c)
+	s.LoggingSuite.TearDownTest(c)
+}
+
+func (s *badBuildSuite) TestBundleToolsBadBuild(c *gc.C) {
+	// Test that original bundleTools Func fails as expected
+	vers, sha256Hash, err := bundleTools(c)
+	c.Assert(vers, gc.DeepEquals, version.Binary{})
+	c.Assert(sha256Hash, gc.Equals, "")
+	c.Assert(err, gc.ErrorMatches, `build command "go" failed: exit status 1; `)
+
+	s.PatchValue(&envtools.BundleTools, toolstesting.GetMockBundleTools(c))
+
+	// Test that BundleTools func passes after it is
+	// mocked out
+	vers, sha256Hash, err = bundleTools(c)
+	c.Assert(err, gc.IsNil)
+	c.Assert(vers.Number, gc.Equals, version.Current.Number)
+	c.Assert(sha256Hash, gc.Equals, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855")
+}
+
+func (s *badBuildSuite) TestUploadToolsBadBuild(c *gc.C) {
+	// Test that original Upload Func fails as expected
+	t, err := sync.Upload(s.env.Storage(), nil)
+	c.Assert(t, gc.IsNil)
+	c.Assert(err, gc.ErrorMatches, `build command "go" failed: exit status 1; `)
+
+	// Test that Upload func passes after BundleTools func is mocked out
+	s.PatchValue(&envtools.BundleTools, toolstesting.GetMockBundleTools(c))
+	t, err = sync.Upload(s.env.Storage(), nil)
+	c.Assert(err, gc.IsNil)
+	c.Assert(t.Version, gc.Equals, version.Current)
+	c.Assert(t.URL, gc.Not(gc.Equals), "")
+}
+
+func (s *badBuildSuite) TestBuildToolsBadBuild(c *gc.C) {
+	// Test that original BuildToolsTarball fails
+	builtTools, err := sync.BuildToolsTarball(nil)
+	c.Assert(err, gc.ErrorMatches, `build command "go" failed: exit status 1; `)
+	c.Assert(builtTools, gc.IsNil)
+
+	// Test that BuildToolsTarball func passes after BundleTools func is
+	// mocked out
+	s.PatchValue(&envtools.BundleTools, toolstesting.GetMockBundleTools(c))
+	builtTools, err = sync.BuildToolsTarball(nil)
+	c.Assert(builtTools.Version, gc.Equals, version.Current)
+	c.Assert(err, gc.IsNil)
+}
+
+func (s *uploadSuite) TestMockBundleTools(c *gc.C) {
+	var (
+		writer       io.Writer
+		forceVersion *version.Number
+		n            int
+		p            bytes.Buffer
+	)
+	p.WriteString("Hello World")
+
+	s.PatchValue(&envtools.BundleTools, func(writerArg io.Writer, forceVersionArg *version.Number) (vers version.Binary, sha256Hash string, err error) {
+		writer = writerArg
+		n, err = writer.Write(p.Bytes())
+		c.Assert(err, gc.IsNil)
+		forceVersion = forceVersionArg
+		return
+	})
+
+	_, err := sync.BuildToolsTarball(&version.Current.Number)
+	c.Assert(err, gc.IsNil)
+	c.Assert(*forceVersion, gc.Equals, version.Current.Number)
+	c.Assert(writer, gc.NotNil)
+	c.Assert(n, gc.Equals, len(p.Bytes()))
+}
+
+func (s *uploadSuite) TestMockBuildTools(c *gc.C) {
+	s.PatchValue(&version.Current, version.MustParseBinary("1.9.1-trusty-amd64"))
+	buildToolsFunc := toolstesting.GetMockBuildTools(c)
+	builtTools, err := buildToolsFunc(nil)
+	c.Assert(err, gc.IsNil)
+
+	builtTools.Dir = ""
+
+	expectedBuiltTools := &sync.BuiltTools{
+		StorageName: "name",
+		Version:     version.Current,
+		Size:        127,
+		Sha256Hash:  "6a19d08ca4913382ca86508aa38eb8ee5b9ae2d74333fe8d862c0f9e29b82c39",
+	}
+	c.Assert(builtTools, gc.DeepEquals, expectedBuiltTools)
+
+	vers := version.MustParseBinary("1.5.3-trusty-amd64")
+	builtTools, err = buildToolsFunc(&vers.Number)
+	c.Assert(err, gc.IsNil)
+	builtTools.Dir = ""
+	expectedBuiltTools = &sync.BuiltTools{
+		StorageName: "name",
+		Version:     vers,
+		Size:        127,
+		Sha256Hash:  "cad8ccedab8f26807ff379ddc2f2f78d9a7cac1276e001154cee5e39b9ddcc38",
+	}
+	c.Assert(builtTools, gc.DeepEquals, expectedBuiltTools)
+}

=== modified file 'environs/tools/build.go'
--- environs/tools/build.go	2014-04-03 04:17:49 +0000
+++ environs/tools/build.go	2014-05-20 00:26:58 +0000
@@ -18,10 +18,10 @@
 	"launchpad.net/juju-core/version"
 )
 
-// archive writes the executable files found in the given directory in
+// Archive writes the executable files found in the given directory in
 // gzipped tar format to w, returning the SHA256 hash of the resulting file.
 // An error is returned if an entry inside dir is not a regular executable file.
-func archive(w io.Writer, dir string) (string, error) {
+func Archive(w io.Writer, dir string) (string, error) {
 	entries, err := ioutil.ReadDir(dir)
 	if err != nil {
 		return "", err
@@ -198,11 +198,18 @@
 	return nil
 }
 
+// BundleToolsFunc is a function which can bundle all the current juju tools
+// in gzipped tar format to the given writer.
+type BundleToolsFunc func(w io.Writer, forceVersion *version.Number) (version.Binary, string, error)
+
+// Override for testing.
+var BundleTools BundleToolsFunc = bundleTools
+
 // BundleTools bundles all the current juju tools in gzipped tar
 // format to the given writer.
 // If forceVersion is not nil, a FORCE-VERSION file is included in
 // the tools bundle so it will lie about its current version number.
-func BundleTools(w io.Writer, forceVersion *version.Number) (tvers version.Binary, sha256Hash string, err error) {
+func bundleTools(w io.Writer, forceVersion *version.Number) (tvers version.Binary, sha256Hash string, err error) {
 	dir, err := ioutil.TempDir("", "juju-tools")
 	if err != nil {
 		return version.Binary{}, "", err
@@ -232,7 +239,7 @@
 	if err != nil {
 		return version.Binary{}, "", fmt.Errorf("invalid version %q printed by jujud", tvs)
 	}
-	sha256Hash, err = archive(w, dir)
+	sha256Hash, err = Archive(w, dir)
 	if err != nil {
 		return version.Binary{}, "", err
 	}

=== modified file 'environs/tools/simplestreams_test.go'
--- environs/tools/simplestreams_test.go	2014-03-21 03:27:16 +0000
+++ environs/tools/simplestreams_test.go	2014-05-20 00:26:58 +0000
@@ -22,7 +22,7 @@
 	sstesting "launchpad.net/juju-core/environs/simplestreams/testing"
 	"launchpad.net/juju-core/environs/storage"
 	"launchpad.net/juju-core/environs/tools"
-	ttesting "launchpad.net/juju-core/environs/tools/testing"
+	toolstesting "launchpad.net/juju-core/environs/tools/testing"
 	"launchpad.net/juju-core/testing/testbase"
 	coretools "launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/utils"
@@ -332,7 +332,7 @@
 	c.Assert(err, gc.IsNil)
 	err = tools.MergeAndWriteMetadata(writer, toolsList, tools.DoNotWriteMirrors)
 	c.Assert(err, gc.IsNil)
-	metadata := ttesting.ParseMetadataFromDir(c, dir, false)
+	metadata := toolstesting.ParseMetadataFromDir(c, dir, false)
 	assertMetadataMatches(c, dir, toolsList, metadata)
 }
 
@@ -342,7 +342,7 @@
 		"2.0.1-raring-amd64",
 	}
 	dir := c.MkDir()
-	ttesting.MakeTools(c, dir, "releases", versionStrings)
+	toolstesting.MakeTools(c, dir, "releases", versionStrings)
 
 	toolsList := coretools.List{
 		{
@@ -364,7 +364,7 @@
 	}
 	err = tools.MergeAndWriteMetadata(writer, toolsList, writeMirrors)
 	c.Assert(err, gc.IsNil)
-	metadata := ttesting.ParseMetadataFromDir(c, dir, withMirrors)
+	metadata := toolstesting.ParseMetadataFromDir(c, dir, withMirrors)
 	assertMetadataMatches(c, dir, toolsList, metadata)
 }
 
@@ -404,7 +404,7 @@
 	err = tools.MergeAndWriteMetadata(writer, newToolsList, tools.DoNotWriteMirrors)
 	c.Assert(err, gc.IsNil)
 	requiredToolsList := append(existingToolsList, newToolsList[1])
-	metadata := ttesting.ParseMetadataFromDir(c, dir, false)
+	metadata := toolstesting.ParseMetadataFromDir(c, dir, false)
 	assertMetadataMatches(c, dir, requiredToolsList, metadata)
 }
 
@@ -557,7 +557,7 @@
 func (*metadataHelperSuite) TestResolveMetadata(c *gc.C) {
 	var versionStrings = []string{"1.2.3-precise-amd64"}
 	dir := c.MkDir()
-	ttesting.MakeTools(c, dir, "releases", versionStrings)
+	toolstesting.MakeTools(c, dir, "releases", versionStrings)
 	toolsList := coretools.List{{
 		Version: version.MustParseBinary(versionStrings[0]),
 		Size:    123,

=== modified file 'environs/tools/testing/testing.go'
--- environs/tools/testing/testing.go	2014-05-13 23:18:30 +0000
+++ environs/tools/testing/testing.go	2014-05-20 00:26:58 +0000
@@ -5,7 +5,9 @@
 
 import (
 	"bytes"
+	"crypto/sha256"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"path"
@@ -20,7 +22,9 @@
 	"launchpad.net/juju-core/environs/filestorage"
 	"launchpad.net/juju-core/environs/simplestreams"
 	"launchpad.net/juju-core/environs/storage"
+	"launchpad.net/juju-core/environs/sync"
 	"launchpad.net/juju-core/environs/tools"
+	coretesting "launchpad.net/juju-core/testing"
 	coretools "launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/utils"
 	"launchpad.net/juju-core/utils/set"
@@ -28,6 +32,44 @@
 	"launchpad.net/juju-core/version/ubuntu"
 )
 
+func GetMockBundleTools(c *gc.C) tools.BundleToolsFunc {
+	return func(w io.Writer, forceVersion *version.Number) (vers version.Binary, sha256Hash string, err error) {
+		vers = version.Current
+		if forceVersion != nil {
+			vers.Number = *forceVersion
+		}
+		sha256Hash = fmt.Sprintf("%x", sha256.New().Sum(nil))
+		return vers, sha256Hash, err
+	}
+}
+
+// GetMockBuildTools returns a sync.BuildToolsTarballFunc implementation which generates
+// a fake tools tarball.
+func GetMockBuildTools(c *gc.C) sync.BuildToolsTarballFunc {
+	return func(forceVersion *version.Number) (*sync.BuiltTools, error) {
+		vers := version.Current
+		if forceVersion != nil {
+			vers.Number = *forceVersion
+		}
+
+		tgz, checksum := coretesting.TarGz(
+			coretesting.NewTarFile("jujud", 0777, "jujud contents "+vers.String()))
+
+		toolsDir, err := ioutil.TempDir("", "juju-tools")
+		c.Assert(err, gc.IsNil)
+		name := "name"
+		ioutil.WriteFile(filepath.Join(toolsDir, name), tgz, 0777)
+
+		return &sync.BuiltTools{
+			Dir:         toolsDir,
+			StorageName: name,
+			Version:     vers,
+			Size:        int64(len(tgz)),
+			Sha256Hash:  checksum,
+		}, nil
+	}
+}
+
 // MakeTools creates some fake tools with the given version strings.
 func MakeTools(c *gc.C, metadataDir, subdir string, versionStrings []string) coretools.List {
 	return makeTools(c, metadataDir, subdir, versionStrings, false)

=== modified file 'environs/tools/tools_test.go'
--- environs/tools/tools_test.go	2014-05-13 04:30:48 +0000
+++ environs/tools/tools_test.go	2014-05-20 00:26:58 +0000
@@ -18,7 +18,7 @@
 	"launchpad.net/juju-core/environs/configstore"
 	envtesting "launchpad.net/juju-core/environs/testing"
 	envtools "launchpad.net/juju-core/environs/tools"
-	ttesting "launchpad.net/juju-core/environs/tools/testing"
+	toolstesting "launchpad.net/juju-core/environs/tools/testing"
 	"launchpad.net/juju-core/provider/dummy"
 	"launchpad.net/juju-core/testing"
 	"launchpad.net/juju-core/testing/testbase"
@@ -82,11 +82,11 @@
 }
 
 func (s *SimpleStreamsToolsSuite) uploadCustom(c *gc.C, verses ...version.Binary) map[version.Binary]string {
-	return ttesting.UploadToDirectory(c, s.customToolsDir, verses...)
+	return toolstesting.UploadToDirectory(c, s.customToolsDir, verses...)
 }
 
 func (s *SimpleStreamsToolsSuite) uploadPublic(c *gc.C, verses ...version.Binary) map[version.Binary]string {
-	return ttesting.UploadToDirectory(c, s.publicToolsDir, verses...)
+	return toolstesting.UploadToDirectory(c, s.publicToolsDir, verses...)
 }
 
 func (s *SimpleStreamsToolsSuite) resetEnv(c *gc.C, attrs map[string]interface{}) {
@@ -180,7 +180,7 @@
 
 func (s *SimpleStreamsToolsSuite) TestFindToolsInControlBucket(c *gc.C) {
 	s.reset(c, nil)
-	custom := ttesting.UploadToStorage(c, s.env.Storage(), envtesting.V110p...)
+	custom := toolstesting.UploadToStorage(c, s.env.Storage(), envtesting.V110p...)
 	s.uploadPublic(c, envtesting.VAll...)
 	actual, err := envtools.FindTools(s.env, 1, 1, coretools.Filter{}, envtools.DoNotAllowRetry)
 	c.Assert(err, gc.IsNil)

=== modified file 'state/apiserver/client/client_test.go'
--- state/apiserver/client/client_test.go	2014-05-13 04:30:48 +0000
+++ state/apiserver/client/client_test.go	2014-05-20 00:26:58 +0000
@@ -21,7 +21,7 @@
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/environs/manual"
 	envstorage "launchpad.net/juju-core/environs/storage"
-	ttesting "launchpad.net/juju-core/environs/tools/testing"
+	toolstesting "launchpad.net/juju-core/environs/tools/testing"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/provider/dummy"
 	"launchpad.net/juju-core/state"
@@ -1607,7 +1607,7 @@
 	result, err := s.APIState.Client().FindTools(2, -1, "", "")
 	c.Assert(err, gc.IsNil)
 	c.Assert(result.Error, jc.Satisfies, params.IsCodeNotFound)
-	ttesting.UploadToStorage(c, s.Conn.Environ.Storage(), version.MustParseBinary("2.12.0-precise-amd64"))
+	toolstesting.UploadToStorage(c, s.Conn.Environ.Storage(), version.MustParseBinary("2.12.0-precise-amd64"))
 	result, err = s.APIState.Client().FindTools(2, 12, "precise", "amd64")
 	c.Assert(err, gc.IsNil)
 	c.Assert(result.Error, gc.IsNil)

=== modified file 'state/apiserver/tools_test.go'
--- state/apiserver/tools_test.go	2014-03-13 05:57:38 +0000
+++ state/apiserver/tools_test.go	2014-05-20 00:26:58 +0000
@@ -12,7 +12,7 @@
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/environs/tools"
-	ttesting "launchpad.net/juju-core/environs/tools/testing"
+	toolstesting "launchpad.net/juju-core/environs/tools/testing"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api/params"
 	coretools "launchpad.net/juju-core/tools"
@@ -106,7 +106,7 @@
 	localStorage := c.MkDir()
 	vers := version.MustParseBinary("1.9.0-quantal-amd64")
 	versionStrings := []string{vers.String()}
-	expectedTools := ttesting.MakeToolsWithCheckSum(c, localStorage, "releases", versionStrings)
+	expectedTools := toolstesting.MakeToolsWithCheckSum(c, localStorage, "releases", versionStrings)
 
 	// Now try uploading them.
 	toolsFile := tools.StorageName(vers)
@@ -136,7 +136,7 @@
 	localStorage := c.MkDir()
 	vers := version.MustParseBinary("1.9.0-quantal-amd64")
 	versionStrings := []string{vers.String()}
-	expectedTools := ttesting.MakeToolsWithCheckSum(c, localStorage, "releases", versionStrings)
+	expectedTools := toolstesting.MakeToolsWithCheckSum(c, localStorage, "releases", versionStrings)
 
 	// Now try uploading them.
 	toolsFile := tools.StorageName(vers)