Skip to content
This repository has been archived by the owner on Sep 24, 2021. It is now read-only.

Add container test #212

Merged
merged 1 commit into from
Aug 26, 2017
Merged

Add container test #212

merged 1 commit into from
Aug 26, 2017

Conversation

Lily922
Copy link
Contributor

@Lily922 Lily922 commented Aug 20, 2017

No description provided.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 20, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@Lily922
Copy link
Contributor Author

Lily922 commented Aug 20, 2017

I'm not sure if this test is designed correctly, you can just think of it as a sample.

@feiskyer
Copy link
Contributor

@LilyFaFa Thanks very much. Please refer docker fake client for adding a fakeClient. Generally, it should

  • implements client interface (as you already done)
  • contain a few set interfaces, e.g. SetContainers, SetImages, so other tests could update the internal state of fakeClient and revisit whether the state is expected after some operations
  • list of called interfaces, so that tests could check whether expected interfaces are called

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 25, 2017
@Lily922
Copy link
Contributor Author

Lily922 commented Aug 25, 2017

@feiskyer Thank you for your advice,I have updated the code by referring to the item in the link,this is the new code.

@feiskyer
Copy link
Contributor

@LilyFaFa Thanks. Seems the PR includes some changes already merged. Could you make a rebase toward master?

@Lily922
Copy link
Contributor Author

Lily922 commented Aug 26, 2017

@feiskyer Please have a look ,is this time ok?

@feiskyer
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2017
@feiskyer
Copy link
Contributor

@LilyFaFa Thanks a lot

@feiskyer feiskyer merged commit f8a7f90 into kubernetes-retired:master Aug 26, 2017
@feiskyer feiskyer mentioned this pull request Sep 27, 2017
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants