Skip to content

validate_test: add TestCheckMandatoryFields#554

Merged
liangchenye merged 2 commits intoopencontainers:masterfrom
zhouhao3:test-CheckMandatoryFields
Feb 7, 2018
Merged

validate_test: add TestCheckMandatoryFields#554
liangchenye merged 2 commits intoopencontainers:masterfrom
zhouhao3:test-CheckMandatoryFields

Conversation

@zhouhao3
Copy link

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

Signed-off-by: Zhou Hao zhouhao@cn.fujitsu.com

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

Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com>
@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?

Signed-off-by: Zhou Hao <zhouhao@cn.fujitsu.com>
@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