Merge lp://staging/~rogpeppe/juju-core/559-fix-err-deepequals into lp://staging/~go-bot/juju-core/trunk
Proposed by
Roger Peppe
| Status: | Work in progress |
|---|---|
| Proposed branch: | lp://staging/~rogpeppe/juju-core/559-fix-err-deepequals |
| Merge into: | lp://staging/~go-bot/juju-core/trunk |
| Diff against target: |
99 lines (+14/-12) 4 files modified
state/machine_test.go (+4/-2) testing/locking_test.go (+6/-5) utils/apt_test.go (+2/-4) worker/instancepoller/aggregate_test.go (+2/-1) |
| To merge this branch: | bzr merge lp://staging/~rogpeppe/juju-core/559-fix-err-deepequals |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Juju Engineering | Pending | ||
|
Review via email:
|
|||
Commit message
various: fix error DeepEquals comparisons
We should not be comparing errors with DeepEquals
in general - they have private fields, and this will
break when using adding trace information with errgo.
Description of the change
various: fix error DeepEquals comparisons
We should not be comparing errors with DeepEquals
in general - they have private fields, and this will
break when using adding trace information with errgo.
To post a comment you must log in.
Revision history for this message
| Roger Peppe (rogpeppe) wrote | # |
Download full text (4.7 KiB)
Revision history for this message
| William Reade (fwereade) wrote | # |
Revision history for this message
| Go Bot (go-bot) wrote | # |
Download full text (12.8 KiB)
Revision history for this message
| John A Meinel (jameinel) wrote | # |
Unmerged revisions
- 2654. By Roger Peppe
-
merge trunk
- 2653. By Roger Peppe
-
various: fix error deepequal comparisons in tests
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
Reviewers: mp+216540_
Message:
Please take a look.
Description:
various: fix error DeepEquals comparisons
We should not be comparing errors with DeepEquals
in general - they have private fields, and this will
break when using adding trace information with errgo.
https:/
(do not edit description out of merge proposal)
Please review this at https:/
Affected files (+16, -12 lines):
A [revision details]
M state/machine_
M testing/
M utils/apt_test.go
M worker/
Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-
+New revision: <email address hidden>
Index: state/machine_
=== modified file 'state/
--- state/machine_
+++ state/machine_
@@ -137,7 +137,8 @@
c.Assert(err, gc.FitsTypeOf, &state.
c.Assert(err, gc.ErrorMatches, `machine 1 is hosting
containers "1/lxc/0"`)
err1 := s.machine.
- c.Assert(err1, gc.DeepEquals, err)
+ c.Assert(err1, gc.FitsTypeOf, &state.
+ c.Assert(err1, gc.ErrorMatches, `machine 1 is hosting
containers "1/lxc/0"`)
c.Assert(
}
@@ -152,7 +153,8 @@
c.Assert(err, gc.FitsTypeOf, &state.
c.Assert(err, gc.ErrorMatches, `machine 1 has unit "wordpress/0"
assigned`)
err1 := s.machine.
- c.Assert(err1, gc.DeepEquals, err)
+ c.Assert(err, gc.FitsTypeOf, &state.
+ c.Assert(err, gc.ErrorMatches, `machine 1 has unit "wordpress/0"
assigned`)
c.Assert(
// Once no unit is assigned, lifecycle can advance.
Index: testing/
=== modified file 'testing/
--- testing/
+++ testing/
@@ -4,7 +4,6 @@
package testing
import (
- "errors"
"sync"
gc "launchpad.
@@ -29,8 +28,9 @@
function := func() {}
c.Check(
func() { TestLockingFunc
- gc.Panics,
- errors.
+ gc.PanicMatches,
+ "function did not obey lock",
+ )
}
func (LockingSuite) TestTestLocking
*gc.C) {
@@ -41,6 +41,7 @@
}
c.Check(
func() { TestLockingFunc
- gc.Panics,
- errors.
+ gc.PanicMatches,
+ "function did not release lock",
+ )
}
Index: utils/apt_test.go
=== modified file 'utils/apt_test.go'
--- utils/apt_test.go 2014-03-21 05:07:45 +0000
+++ utils/apt_test.go 2014-04-20 12:40:45 +0000
@@ -42,10 +42,9 @@
func (s *AptSuite) TestAptGetError(c *gc.C) {
const expected = `E: frobnicator failure detected`
cmdError := fmt.Errorf("error")
-...