Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate_test: add TestCheckMandatoryFields #554

Merged

Conversation

zhouhao3
Copy link

Because there are so many cases, I only chose part of the test.

Signed-off-by: Zhou Hao [email protected]

Because there are so many cases, I only chose part of the test.

Signed-off-by: Zhou Hao <[email protected]>
@zhouhao3 zhouhao3 force-pushed the test-CheckMandatoryFields branch from 035a445 to 8769602 Compare January 18, 2018 06:36
@zhouhao3
Copy link
Author

zhouhao3 commented Feb 2, 2018

@liangchenye PTAL

1 similar comment
@zhouhao3
Copy link
Author

zhouhao3 commented Feb 7, 2018

@liangchenye PTAL

@liangchenye
Copy link
Member

Looking into this.
I'm checking if the unit test cover all cases of this function.

@liangchenye
Copy link
Member

It looks good. I just found the original function does not handle one case -- the 'v.spec == nil'.
Can you fix it and add the related unit test?

@zhouhao3
Copy link
Author

zhouhao3 commented Feb 7, 2018

updated, thanks. PTAL.

@liangchenye
Copy link
Member

liangchenye commented Feb 7, 2018

LGTM

Approved with PullApprove

@liangchenye liangchenye merged commit 21c58ab into opencontainers:master Feb 7, 2018
@zhouhao3 zhouhao3 deleted the test-CheckMandatoryFields branch February 8, 2018 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants