juju-core

Merge lp:~frankban/juju-core/utils-testing into lp:~go-bot/juju-core/trunk

  1. utils-testing
  2. Merge into trunk
Proposed by Francesco Banconi
Status: Rejected
Rejected by: Francesco Banconi
Proposed branch: lp:~frankban/juju-core/utils-testing
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 537 lines (+80/-66)
20 files modified
utils/apt/apt_test.go (+12/-2)
utils/command_test.go (+2/-2)
utils/exec/exec_test.go (+1/-4)
utils/fslock/fslock_test.go (+3/-2)
utils/gomaxprocs_test.go (+3/-3)
utils/isubuntu_test.go (+2/-2)
utils/proxy/package_test.go (+2/-2)
utils/proxy/proxy_test.go (+2/-2)
utils/registry/registry_test.go (+1/-4)
utils/shell/script_test.go (+1/-4)
utils/ssh/fingerprint_test.go (+1/-4)
utils/ssh/generate_test.go (+1/-4)
utils/ssh/run_test.go (+8/-7)
utils/ssh/ssh_gocrypto_test.go (+3/-3)
utils/ssh/ssh_test.go (+3/-3)
utils/tailer/package_test.go (+14/-0)
utils/tailer/tailer_test.go (+4/-8)
utils/voyeur/package_test.go (+14/-0)
utils/voyeur/value_test.go (+1/-8)
utils/zip/zip_test.go (+2/-2)
To merge this branch: bzr merge lp:~frankban/juju-core/utils-testing
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+220991@code.launchpad.net

Description of the change

Do not use BaseSuite in utils if not required.

Another incremental step for decoupling
utils stuff from juju-core related code.

https://codereview.appspot.com/101760045/

To post a comment you must log in.
Revision history for this message
Francesco Banconi (frankban) wrote :

Reviewers: mp+220991_code.launchpad.net,

Message:
Please take a look.

Description:
Do not use BaseSuite in utils if not required.

Another incremental step for decoupling
utils stuff from juju-core related code.

https://code.launchpad.net/~frankban/juju-core/utils-testing/+merge/220991

(do not edit description out of merge proposal)

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

Affected files (+82, -66 lines):
   A [revision details]
   M utils/apt/apt_test.go
   M utils/command_test.go
   M utils/exec/exec_test.go
   M utils/fslock/fslock_test.go
   M utils/gomaxprocs_test.go
   M utils/isubuntu_test.go
   M utils/proxy/package_test.go
   M utils/proxy/proxy_test.go
   M utils/registry/registry_test.go
   M utils/shell/script_test.go
   M utils/ssh/fingerprint_test.go
   M utils/ssh/generate_test.go
   M utils/ssh/run_test.go
   M utils/ssh/ssh_gocrypto_test.go
   M utils/ssh/ssh_test.go
   A utils/tailer/package_test.go
   M utils/tailer/tailer_test.go
   A utils/voyeur/package_test.go
   M utils/voyeur/value_test.go
   M utils/zip/zip_test.go

Revision history for this message
William Reade (fwereade) wrote :

I'm rather -1 on this. I see very little that's juju-specific in
TestBase -- the only issue I see is that it clears out only *some* env
vars, not *all* of them, as (I reckon) we ought to do as a matter of
course anyway.

Chat live?

https://codereview.appspot.com/101760045/diff/1/utils/apt/apt_test.go
File utils/apt/apt_test.go (right):

https://codereview.appspot.com/101760045/diff/1/utils/apt/apt_test.go#newcode33
utils/apt/apt_test.go:33: s.LoggingSuite.SetUpTest(c)
doesn't CleanupSuite have SetUpTest/TearDownTest? I feel like it
should.,..

https://codereview.appspot.com/101760045/

Unmerged revisions

2801. By Francesco Banconi

Remove BaseSuite from utils/ssh.

2800. By Francesco Banconi

Handle proxy, tailer and voyeur.

2799. By Francesco Banconi

Handle registry, shell and zip.

2798. By Francesco Banconi

Handle exec, fslock and proxy.

2797. By Francesco Banconi

Remove BaseSuite in utils/apt

2796. By Francesco Banconi

Remove BaseSuite from utils.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'utils/apt/apt_test.go'
2--- utils/apt/apt_test.go 2014-05-26 10:47:42 +0000
3+++ utils/apt/apt_test.go 2014-05-26 16:11:42 +0000
4@@ -9,10 +9,11 @@
5 "path/filepath"
6 stdtesting "testing"
7
8+ "github.com/juju/testing"
9 jc "github.com/juju/testing/checkers"
10 gc "launchpad.net/gocheck"
11
12- "launchpad.net/juju-core/testing"
13+ "launchpad.net/juju-core/testing/testbase"
14 "launchpad.net/juju-core/utils/apt"
15 "launchpad.net/juju-core/utils/proxy"
16 )
17@@ -22,11 +23,20 @@
18 }
19
20 type AptSuite struct {
21- testing.BaseSuite
22+ testing.CleanupSuite
23+ testbase.LoggingSuite
24 }
25
26 var _ = gc.Suite(&AptSuite{})
27
28+func (s *AptSuite) SetUpTest(c *gc.C) {
29+ s.LoggingSuite.SetUpTest(c)
30+}
31+
32+func (s *AptSuite) TearDownTest(c *gc.C) {
33+ s.LoggingSuite.TearDownTest(c)
34+}
35+
36 func (s *AptSuite) TestOnePackage(c *gc.C) {
37 cmdChan := s.HookCommandOutput(&apt.CommandOutput, []byte{}, nil)
38 err := apt.GetInstall("test-package")
39
40=== modified file 'utils/command_test.go'
41--- utils/command_test.go 2014-05-20 04:27:02 +0000
42+++ utils/command_test.go 2014-05-26 16:11:42 +0000
43@@ -7,9 +7,9 @@
44 "io/ioutil"
45 "path/filepath"
46
47+ "github.com/juju/testing"
48 gc "launchpad.net/gocheck"
49
50- "launchpad.net/juju-core/testing"
51 "launchpad.net/juju-core/utils"
52 )
53
54@@ -24,7 +24,7 @@
55 }
56
57 type commandSuite struct {
58- testing.BaseSuite
59+ testing.CleanupSuite
60 }
61
62 var _ = gc.Suite(&commandSuite{})
63
64=== modified file 'utils/exec/exec_test.go'
65--- utils/exec/exec_test.go 2014-05-20 04:27:02 +0000
66+++ utils/exec/exec_test.go 2014-05-26 16:11:42 +0000
67@@ -7,13 +7,10 @@
68 jc "github.com/juju/testing/checkers"
69 gc "launchpad.net/gocheck"
70
71- "launchpad.net/juju-core/testing"
72 "launchpad.net/juju-core/utils/exec"
73 )
74
75-type execSuite struct {
76- testing.BaseSuite
77-}
78+type execSuite struct{}
79
80 var _ = gc.Suite(&execSuite{})
81
82
83=== modified file 'utils/fslock/fslock_test.go'
84--- utils/fslock/fslock_test.go 2014-05-20 04:27:02 +0000
85+++ utils/fslock/fslock_test.go 2014-05-26 16:11:42 +0000
86@@ -12,6 +12,7 @@
87 "sync/atomic"
88 "time"
89
90+ "github.com/juju/testing"
91 gc "launchpad.net/gocheck"
92 "launchpad.net/tomb"
93
94@@ -20,14 +21,14 @@
95 )
96
97 type fslockSuite struct {
98- coretesting.BaseSuite
99+ testing.CleanupSuite
100 lockDelay time.Duration
101 }
102
103 var _ = gc.Suite(&fslockSuite{})
104
105 func (s *fslockSuite) SetUpSuite(c *gc.C) {
106- s.BaseSuite.SetUpSuite(c)
107+ s.CleanupSuite.SetUpSuite(c)
108 s.PatchValue(&fslock.LockWaitDelay, 1*time.Millisecond)
109 }
110
111
112=== modified file 'utils/gomaxprocs_test.go'
113--- utils/gomaxprocs_test.go 2014-05-20 04:27:02 +0000
114+++ utils/gomaxprocs_test.go 2014-05-26 16:11:42 +0000
115@@ -6,14 +6,14 @@
116 import (
117 "os"
118
119+ "github.com/juju/testing"
120 gc "launchpad.net/gocheck"
121
122- "launchpad.net/juju-core/testing"
123 "launchpad.net/juju-core/utils"
124 )
125
126 type gomaxprocsSuite struct {
127- testing.BaseSuite
128+ testing.CleanupSuite
129 setmaxprocs chan int
130 numCPUResponse int
131 setMaxProcs int
132@@ -22,7 +22,7 @@
133 var _ = gc.Suite(&gomaxprocsSuite{})
134
135 func (s *gomaxprocsSuite) SetUpTest(c *gc.C) {
136- s.BaseSuite.SetUpTest(c)
137+ s.CleanupSuite.SetUpTest(c)
138 // always stub out GOMAXPROCS so we don't actually change anything
139 s.numCPUResponse = 2
140 s.setMaxProcs = -1
141
142=== modified file 'utils/isubuntu_test.go'
143--- utils/isubuntu_test.go 2014-05-23 11:58:14 +0000
144+++ utils/isubuntu_test.go 2014-05-26 16:11:42 +0000
145@@ -6,15 +6,15 @@
146 import (
147 "fmt"
148
149+ "github.com/juju/testing"
150 jc "github.com/juju/testing/checkers"
151 gc "launchpad.net/gocheck"
152
153- "launchpad.net/juju-core/testing"
154 "launchpad.net/juju-core/utils"
155 )
156
157 type IsUbuntuSuite struct {
158- testing.BaseSuite
159+ testing.CleanupSuite
160 }
161
162 var _ = gc.Suite(&IsUbuntuSuite{})
163
164=== modified file 'utils/proxy/package_test.go'
165--- utils/proxy/package_test.go 2014-05-26 12:45:10 +0000
166+++ utils/proxy/package_test.go 2014-05-26 16:11:42 +0000
167@@ -4,11 +4,11 @@
168 package proxy_test
169
170 import (
171- stdtesting "testing"
172+ "testing"
173
174 gc "launchpad.net/gocheck"
175 )
176
177-func TestPackage(t *stdtesting.T) {
178+func TestPackage(t *testing.T) {
179 gc.TestingT(t)
180 }
181
182=== modified file 'utils/proxy/proxy_test.go'
183--- utils/proxy/proxy_test.go 2014-05-26 12:45:10 +0000
184+++ utils/proxy/proxy_test.go 2014-05-26 16:11:42 +0000
185@@ -6,14 +6,14 @@
186 import (
187 "os"
188
189+ "github.com/juju/testing"
190 gc "launchpad.net/gocheck"
191
192- "launchpad.net/juju-core/testing"
193 "launchpad.net/juju-core/utils/proxy"
194 )
195
196 type proxySuite struct {
197- testing.BaseSuite
198+ testing.CleanupSuite
199 }
200
201 var _ = gc.Suite(&proxySuite{})
202
203=== modified file 'utils/registry/registry_test.go'
204--- utils/registry/registry_test.go 2014-05-23 11:24:54 +0000
205+++ utils/registry/registry_test.go 2014-05-26 16:11:42 +0000
206@@ -10,13 +10,10 @@
207 jc "github.com/juju/testing/checkers"
208 gc "launchpad.net/gocheck"
209
210- "launchpad.net/juju-core/testing"
211 "launchpad.net/juju-core/utils/registry"
212 )
213
214-type registrySuite struct {
215- testing.BaseSuite
216-}
217+type registrySuite struct{}
218
219 var _ = gc.Suite(&registrySuite{})
220
221
222=== modified file 'utils/shell/script_test.go'
223--- utils/shell/script_test.go 2014-05-20 04:27:02 +0000
224+++ utils/shell/script_test.go 2014-05-26 16:11:42 +0000
225@@ -13,13 +13,10 @@
226
227 gc "launchpad.net/gocheck"
228
229- "launchpad.net/juju-core/testing"
230 "launchpad.net/juju-core/utils/shell"
231 )
232
233-type scriptSuite struct {
234- testing.BaseSuite
235-}
236+type scriptSuite struct{}
237
238 var _ = gc.Suite(&scriptSuite{})
239
240
241=== modified file 'utils/ssh/fingerprint_test.go'
242--- utils/ssh/fingerprint_test.go 2014-05-20 04:27:02 +0000
243+++ utils/ssh/fingerprint_test.go 2014-05-26 16:11:42 +0000
244@@ -6,14 +6,11 @@
245 import (
246 gc "launchpad.net/gocheck"
247
248- "launchpad.net/juju-core/testing"
249 "launchpad.net/juju-core/utils/ssh"
250 sshtesting "launchpad.net/juju-core/utils/ssh/testing"
251 )
252
253-type FingerprintSuite struct {
254- testing.BaseSuite
255-}
256+type FingerprintSuite struct{}
257
258 var _ = gc.Suite(&FingerprintSuite{})
259
260
261=== modified file 'utils/ssh/generate_test.go'
262--- utils/ssh/generate_test.go 2014-05-20 04:27:02 +0000
263+++ utils/ssh/generate_test.go 2014-05-26 16:11:42 +0000
264@@ -11,13 +11,10 @@
265 jc "github.com/juju/testing/checkers"
266 gc "launchpad.net/gocheck"
267
268- coretesting "launchpad.net/juju-core/testing"
269 "launchpad.net/juju-core/utils/ssh"
270 )
271
272-type GenerateSuite struct {
273- coretesting.BaseSuite
274-}
275+type GenerateSuite struct{}
276
277 var _ = gc.Suite(&GenerateSuite{})
278
279
280=== modified file 'utils/ssh/run_test.go'
281--- utils/ssh/run_test.go 2014-05-20 04:27:02 +0000
282+++ utils/ssh/run_test.go 2014-05-26 16:11:42 +0000
283@@ -7,15 +7,16 @@
284 "io/ioutil"
285 "path/filepath"
286
287+ "github.com/juju/testing"
288 jc "github.com/juju/testing/checkers"
289 gc "launchpad.net/gocheck"
290
291- "launchpad.net/juju-core/testing"
292+ coretesting "launchpad.net/juju-core/testing"
293 "launchpad.net/juju-core/utils/ssh"
294 )
295
296 type ExecuteSSHCommandSuite struct {
297- testing.BaseSuite
298+ testing.CleanupSuite
299 testbin string
300 fakessh string
301 }
302@@ -23,7 +24,7 @@
303 var _ = gc.Suite(&ExecuteSSHCommandSuite{})
304
305 func (s *ExecuteSSHCommandSuite) SetUpTest(c *gc.C) {
306- s.BaseSuite.SetUpTest(c)
307+ s.CleanupSuite.SetUpTest(c)
308 s.testbin = c.MkDir()
309 s.fakessh = filepath.Join(s.testbin, "ssh")
310 s.PatchEnvPathPrepend(s.testbin)
311@@ -40,7 +41,7 @@
312 response, err := ssh.ExecuteCommandOnMachine(ssh.ExecParams{
313 Host: "hostname",
314 Command: "sudo apt-get update\nsudo apt-get upgrade",
315- Timeout: testing.ShortWait,
316+ Timeout: coretesting.ShortWait,
317 })
318
319 c.Assert(err, gc.IsNil)
320@@ -56,7 +57,7 @@
321 response, err := ssh.ExecuteCommandOnMachine(ssh.ExecParams{
322 IdentityFile: "identity-file",
323 Host: "hostname",
324- Timeout: testing.ShortWait,
325+ Timeout: coretesting.ShortWait,
326 })
327
328 c.Assert(err, gc.IsNil)
329@@ -70,7 +71,7 @@
330 IdentityFile: "identity-file",
331 Host: "hostname",
332 Command: "ignored",
333- Timeout: testing.ShortWait,
334+ Timeout: coretesting.ShortWait,
335 })
336
337 c.Check(err, gc.ErrorMatches, "command timed out")
338@@ -86,7 +87,7 @@
339 IdentityFile: "identity-file",
340 Host: "hostname",
341 Command: "echo stdout; exit 42",
342- Timeout: testing.ShortWait,
343+ Timeout: coretesting.ShortWait,
344 })
345
346 c.Check(err, gc.IsNil)
347
348=== modified file 'utils/ssh/ssh_gocrypto_test.go'
349--- utils/ssh/ssh_gocrypto_test.go 2014-05-20 04:27:02 +0000
350+++ utils/ssh/ssh_gocrypto_test.go 2014-05-26 16:11:42 +0000
351@@ -14,10 +14,10 @@
352 "sync"
353
354 cryptossh "code.google.com/p/go.crypto/ssh"
355+ "github.com/juju/testing"
356 jc "github.com/juju/testing/checkers"
357 gc "launchpad.net/gocheck"
358
359- "launchpad.net/juju-core/testing"
360 "launchpad.net/juju-core/utils/ssh"
361 )
362
363@@ -79,14 +79,14 @@
364 }
365
366 type SSHGoCryptoCommandSuite struct {
367- testing.BaseSuite
368+ testing.CleanupSuite
369 client ssh.Client
370 }
371
372 var _ = gc.Suite(&SSHGoCryptoCommandSuite{})
373
374 func (s *SSHGoCryptoCommandSuite) SetUpTest(c *gc.C) {
375- s.BaseSuite.SetUpTest(c)
376+ s.CleanupSuite.SetUpTest(c)
377 generateKeyRestorer := overrideGenerateKey(c)
378 s.AddCleanup(func(*gc.C) { generateKeyRestorer.Restore() })
379 client, err := ssh.NewGoCryptoClient()
380
381=== modified file 'utils/ssh/ssh_test.go'
382--- utils/ssh/ssh_test.go 2014-05-20 04:27:02 +0000
383+++ utils/ssh/ssh_test.go 2014-05-26 16:11:42 +0000
384@@ -9,15 +9,15 @@
385 "path/filepath"
386 "strings"
387
388+ "github.com/juju/testing"
389 gc "launchpad.net/gocheck"
390
391 "launchpad.net/juju-core/cmd"
392- "launchpad.net/juju-core/testing"
393 "launchpad.net/juju-core/utils/ssh"
394 )
395
396 type SSHCommandSuite struct {
397- testing.BaseSuite
398+ testing.CleanupSuite
399 testbin string
400 fakessh string
401 fakescp string
402@@ -29,7 +29,7 @@
403 const echoCommandScript = "#!/bin/sh\necho $0 \"$@\" | tee $0.args"
404
405 func (s *SSHCommandSuite) SetUpTest(c *gc.C) {
406- s.BaseSuite.SetUpTest(c)
407+ s.CleanupSuite.SetUpTest(c)
408 s.testbin = c.MkDir()
409 s.fakessh = filepath.Join(s.testbin, "ssh")
410 s.fakescp = filepath.Join(s.testbin, "scp")
411
412=== added file 'utils/tailer/package_test.go'
413--- utils/tailer/package_test.go 1970-01-01 00:00:00 +0000
414+++ utils/tailer/package_test.go 2014-05-26 16:11:42 +0000
415@@ -0,0 +1,14 @@
416+// Copyright 2014 Canonical Ltd.
417+// Licensed under the AGPLv3, see LICENCE file for details.
418+
419+package tailer_test
420+
421+import (
422+ "testing"
423+
424+ gc "launchpad.net/gocheck"
425+)
426+
427+func TestPackage(t *testing.T) {
428+ gc.TestingT(t)
429+}
430
431=== modified file 'utils/tailer/tailer_test.go'
432--- utils/tailer/tailer_test.go 2014-05-20 04:27:02 +0000
433+++ utils/tailer/tailer_test.go 2014-05-26 16:11:42 +0000
434@@ -9,21 +9,17 @@
435 "fmt"
436 "io"
437 "sync"
438- stdtesting "testing"
439 "time"
440
441+ "github.com/juju/testing"
442 gc "launchpad.net/gocheck"
443
444- "launchpad.net/juju-core/testing"
445+ coretesting "launchpad.net/juju-core/testing"
446 "launchpad.net/juju-core/utils/tailer"
447 )
448
449-func Test(t *stdtesting.T) {
450- gc.TestingT(t)
451-}
452-
453 type tailerSuite struct {
454- testing.BaseSuite
455+ testing.CleanupSuite
456 }
457
458 var _ = gc.Suite(&tailerSuite{})
459@@ -410,7 +406,7 @@
460 if len(compare) == 0 {
461 return
462 }
463- timeout := time.After(testing.LongWait)
464+ timeout := time.After(coretesting.LongWait)
465 lines := []string{}
466 for {
467 select {
468
469=== added file 'utils/voyeur/package_test.go'
470--- utils/voyeur/package_test.go 1970-01-01 00:00:00 +0000
471+++ utils/voyeur/package_test.go 2014-05-26 16:11:42 +0000
472@@ -0,0 +1,14 @@
473+// Copyright 2014 Canonical Ltd.
474+// Licensed under the AGPLv3, see LICENCE file for details.
475+
476+package voyeur
477+
478+import (
479+ "testing"
480+
481+ gc "launchpad.net/gocheck"
482+)
483+
484+func TestPackage(t *testing.T) {
485+ gc.TestingT(t)
486+}
487
488=== modified file 'utils/voyeur/value_test.go'
489--- utils/voyeur/value_test.go 2014-05-20 04:27:02 +0000
490+++ utils/voyeur/value_test.go 2014-05-26 16:11:42 +0000
491@@ -5,22 +5,15 @@
492
493 import (
494 "fmt"
495- stdtesting "testing"
496
497 jc "github.com/juju/testing/checkers"
498 gc "launchpad.net/gocheck"
499-
500- "launchpad.net/juju-core/testing"
501 )
502
503-type suite struct {
504- testing.BaseSuite
505-}
506+type suite struct{}
507
508 var _ = gc.Suite(&suite{})
509
510-func Test(t *stdtesting.T) { gc.TestingT(t) }
511-
512 func ExampleWatcher_Next() {
513 v := NewValue(nil)
514
515
516=== modified file 'utils/zip/zip_test.go'
517--- utils/zip/zip_test.go 2014-05-20 04:27:02 +0000
518+++ utils/zip/zip_test.go 2014-05-26 16:11:42 +0000
519@@ -13,16 +13,16 @@
520 "path/filepath"
521 "sort"
522
523+ "github.com/juju/testing"
524 jc "github.com/juju/testing/checkers"
525 gc "launchpad.net/gocheck"
526
527- "launchpad.net/juju-core/testing"
528 ft "launchpad.net/juju-core/testing/filetesting"
529 "launchpad.net/juju-core/utils/zip"
530 )
531
532 type ZipSuite struct {
533- testing.BaseSuite
534+ testing.CleanupSuite
535 }
536
537 var _ = gc.Suite(&ZipSuite{})

Subscribers

People subscribed via source and target branches

to status/vote changes: