[controller] unwrap errors from m3admin #97
Conversation
We were checking against the wrapped errors which caused the placement and namespace controller actions to behave incorrectly.
|
|
||
| placementMock.EXPECT().Get().Return(nil, errors.New("foo")) | ||
| err = controller.EnsurePlacement(cluster) | ||
| assert.Error(t, err) |
There was a problem hiding this comment.
nit: assert.Error(t, controller.EnsurePlacement(cluster))
There was a problem hiding this comment.
Hmm I worry about breaking convention with the style in the rest of our tests. Also personally I like having the error separate, if you're debugging a test it's easier to do something like fmt.Printf("%#v", err) without having to pull it out of the assert
pkg/controller/add_cluster_test.go
Outdated
| err = controller.EnsurePlacement(cluster) | ||
| assert.NoError(t, err) | ||
|
|
||
| placementMock.EXPECT().Get().Return(nil, errors.New("foo")) |
There was a problem hiding this comment.
I would avoid the foo error messages and put in an actual error message. It would make understanding the tests much easier for people like me :)
|
|
||
| placementMock.EXPECT().Get() | ||
| err = controller.EnsurePlacement(cluster) | ||
| assert.NoError(t, err) |
| placementMock.EXPECT().Init(gomock.Any()) | ||
|
|
||
| err = controller.EnsurePlacement(cluster) | ||
| assert.NoError(t, err) |
Codecov Report
@@ Coverage Diff @@
## master #97 +/- ##
=========================================
Coverage ? 67.16%
=========================================
Files ? 26
Lines ? 1864
Branches ? 0
=========================================
Hits ? 1252
Misses ? 511
Partials ? 101Continue to review full report at Codecov.
|
We were checking against the wrapped errors which caused the placement
and namespace controller actions to behave incorrectly.
Fixes #96