add unit tests for package cli/command/secret#32289
Merged
vdemeester merged 1 commit intomoby:masterfrom Apr 7, 2017
Merged
Conversation
adshmh
changed the title
dnephin
added
the
status/failing-ci
dnephin
reviewed
Apr 3, 2017
Member
dnephin
left a comment
dnephin
left a comment
There was a problem hiding this comment.
Thanks! this looks good, but there are some missing comments causing CI to fail
adshmh
force-pushed
the
add-unit-tests-for-cli-command-secret-package
branch
from
4e6c738 to
1771f02
Compare
Contributor
Author
|
Thank you for the review. I fixed the comments that were not written correctly and updated the PR. |
Member
|
Looks like tests are failing @adshmh |
adshmh
force-pushed
the
add-unit-tests-for-cli-command-secret-package
branch
2 times, most recently
from
a45258a to
7c4770b
Compare
Contributor
Author
|
Thank you for the notification. I think it was due to using multiple labels for test objects (Secret) and the order they appeared in the output (interestingly, all unit-tests passed on my test machine all the time). |
vdemeester
approved these changes
Apr 4, 2017
cli/command/secret/create_test.go
Outdated
Member
There was a problem hiding this comment.
It's a nit but I think we could even just use "requires exactly 2 argument(s)" here 👼
Contributor
Author
There was a problem hiding this comment.
Thank you for the review. The PR is now updated (removed references to the command name from the tests expected errors).
cli/command/secret/create_test.go
Outdated
cli/command/secret/remove_test.go
Outdated
Signed-off-by: Arash Deshmeh <[email protected]>
adshmh
force-pushed
the
add-unit-tests-for-cli-command-secret-package
branch
from
7c4770b to
243a8e6
Compare
dnephin
removed
the
status/failing-ci
dnephin
pushed a commit
to dnephin/docker
that referenced
this pull request
Apr 17, 2017
…and-secret-package add unit tests for package cli/command/secret
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds unit tests for all commands under cli/command/secret package (create, list, inspect, and remove). Part of the work on #31217
Signed-off-by: Arash Deshmeh [email protected]
- What I did
Added unit tests for all the commands in package cli/command/secret
- How I did it
Added unit tests and supporting code (builders + test data files in testdata directory) for all the commands in cli/command/secret package (create, ls, inspect, and remove)
- How to verify it
Run the unit tests.
There are no unit tests for the cli/command/secret package in current version (no test files)
With this PR:
$ make test-unit
...
ok github.com/docker/docker/cli/command/secret 0.069s coverage: 93.2% of statements
...
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)