Uh oh!
There was an error while loading. Please reload this page.
Less verbose fake cli - #299
Conversation
Codecov Report
@@ Coverage Diff @@## master #299 +/- ##
=======================================
Coverage 48.68% 48.68% =======================================
Files 186 186 Lines 12416 12416 =======================================
Hits 6045 6045 Misses 5996 5996 Partials 375 375 |
aaronlehmann
commented
Jul 6, 2017
What do you think about functional args for It feels like this PR goes halfway, in that it renames the function but doesn't update most tests to use the new style. Would it be difficult to update the tests in the same PR, to avoid touching all these tests multiple times? |
dnephin
commented
Jul 6, 2017
Difficult no, but time consuming, there are lots of instances. A regex wont work because they are almost always on multiple lines. I'll see about converting a few more. What's bad about touching the files twice?
Works for me, but I don't think it's related to this change. They would save a few lines when assigning new values to these fields. This change is about removing unnecessary lines, not replacing necessary ones with helpers. |
aaronlehmann
commented
Jul 6, 2017
Touching them once would ease review, overall. |
Use byte buffers by default, since that is what is done most of the time. Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
07b20fa to
23c3da0Comparednephin
commented
Jul 11, 2017
Rebased to fix conflicts, and I converted a bunch more tests to use the new function. I was thinking that the rest would just be handled if/when tests are updated. It's not really a high enough priority that we need to convert them all, but making this change now lets us write all the new tests with the less verbose option. It shouldn't really incur any extra review time because they can be changed as part of updating tests. |
| } | ||
| for _, tc := range testCases { | ||
| cli := test.NewFakeCli(&fakeClient{imageLoadFunc: tc.imageLoadFunc}, new(bytes.Buffer)) | ||
| cli := test.NewFakeCliWithOutput(&fakeClient{imageLoadFunc: tc.imageLoadFunc}, new(bytes.Buffer)) |
There was a problem hiding this comment.
Could just be NewFakeCli, but I know you're not trying to exhaustively convert these.
| func TestNewLoadCommandInvalidInput(t *testing.T) { | ||
| expectedError := "open *" | ||
| cmd := NewLoadCommand(test.NewFakeCli(&fakeClient{}, new(bytes.Buffer))) | ||
| cmd := NewLoadCommand(test.NewFakeCliWithOutput(&fakeClient{}, new(bytes.Buffer))) |
There was a problem hiding this comment.
Could just be NewFakeCli, but I know you're not trying to exhaustively convert these.
| func TestNewRemoveCommandAlias(t *testing.T) { | ||
| cmd := newRemoveCommand(test.NewFakeCli(&fakeClient{}, new(bytes.Buffer))) | ||
| cmd := newRemoveCommand(test.NewFakeCliWithOutput(&fakeClient{}, new(bytes.Buffer))) |
There was a problem hiding this comment.
Could just be NewFakeCli, but I know you're not trying to exhaustively convert these.
| } | ||
| for _, tc := range testCases { | ||
| cmd := NewRemoveCommand(test.NewFakeCli(&fakeClient{ | ||
| cmd := NewRemoveCommand(test.NewFakeCliWithOutput(&fakeClient{ |
There was a problem hiding this comment.
Could just be NewFakeCli, but I know you're not trying to exhaustively convert these.
| } | ||
| for _, tc := range testCases { | ||
| cli := test.NewFakeCli(&fakeClient{imageSaveFunc: tc.imageSaveFunc}, new(bytes.Buffer)) | ||
| cli := test.NewFakeCliWithOutput(&fakeClient{imageSaveFunc: tc.imageSaveFunc}, new(bytes.Buffer)) |
There was a problem hiding this comment.
Could just be NewFakeCli, but I know you're not trying to exhaustively convert these.
| for _, tc := range testCases { | ||
| cmd := newListCommand(test.NewFakeCli(&fakeClient{ | ||
| cmd := newListCommand(test.NewFakeCliWithOutput(&fakeClient{ |
There was a problem hiding this comment.
Could just be NewFakeCli, but I know you're not trying to exhaustively convert these.
| configs: allConfigs, | ||
| } | ||
| cmd := newRemoveCommand(test.NewFakeCli(cli, &bytes.Buffer{})) | ||
| cmd := newRemoveCommand(test.NewFakeCliWithOutput(cli, &bytes.Buffer{})) |
There was a problem hiding this comment.
Could just be NewFakeCli, but I know you're not trying to exhaustively convert these.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
23c3da0 to
42a3800Compare
Almost every use of
FakeCliuses abytes.Bufferfor stdout/stderr. Instead of having the caller setup these buffers every time we can default to using a buffer.I updated some tests to remove unnecessary calls to
SetErr()andSetConfigFile(), and a few tests to use the newNewFakeCli(). I've deprecated the only one.