Skip to content

Fix TestRenameInvalidName#23340

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:fix-TestRenameInvalidName
Jun 7, 2016
Merged

Fix TestRenameInvalidName#23340
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:fix-TestRenameInvalidName

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

Change the test back to what it was before e83dad0 (#23290),
and added an extra test-case to check the output if an incorrect number of arguments is passed.

For some reason, this test passed on #23290, but is now failing. It looks like the command executed is docker rename "" newname, including the quotes, which gives a different error than docker rename myname

Change the test back to what it was before
e83dad0

And added an extra test-case to check the
output if an incorrect number of arguments
is passed.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Copy Markdown
Member Author

ping @justincormack @vdemeester ptal

@justincormack
Copy link
Copy Markdown
Contributor

LGTM

@vdemeester
Copy link
Copy Markdown
Member

ah right, probably because of the update on cobra I did.
LGTM assuming it's :gree

@yongtang
Copy link
Copy Markdown
Member

yongtang commented Jun 7, 2016

@thaJeztah
When #23290 was created, there was an issue in cobra so the test is ok.

Now the issue in cobra has been fixed in #23269 Use spf13/cobra for docker import so it triggered the failing tests.

Thanks for taking care of that.

I didn't notice your pull and created another pull request #23341. Will close #23341 shortly.

@thaJeztah
Copy link
Copy Markdown
Member Author

@yongtang no problem, thanks! At least we understand why it failed now

@thaJeztah
Copy link
Copy Markdown
Member Author

I'm merging this; the test passed on all, win2lin was aborted due to a manual trigger

@thaJeztah thaJeztah merged commit a01ae04 into moby:master Jun 7, 2016
@thaJeztah thaJeztah deleted the fix-TestRenameInvalidName branch June 7, 2016 14:40
@vdemeester
Copy link
Copy Markdown
Member

yay \o/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants