Skip to content

Conversation

@xiekeyang
Copy link
Contributor

Test cases should contain required fields, otherwise result will be always failure whether the validated field be legal or not. That is not our expection.
And add a new test case for only required fields, to check the optional and required fields in spec.

Signed-off-by: xiekeyang [email protected]

Signed-off-by: xiekeyang <[email protected]>
"architecture": "amd64",
"os": 123
"os": 123,
"rootfs": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one (and a few others) is still missing architecture and os.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wking

This one (and a few others) is still missing architecture and os.

Excuse me I might not understand your comment. architecture and os seems have been added to all cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, I'm just blind. ceb8cf8 looks good to me.

@xiekeyang
Copy link
Contributor Author

@opencontainers/image-spec-maintainers I think this PR is ready to be merged, PTAL.
cc @stevvooe @vbatts

@xiekeyang xiekeyang changed the title improve config unit test fix bug: config unit test lack necessary fields Nov 25, 2016
@xiekeyang xiekeyang changed the title fix bug: config unit test lack necessary fields fix bug: config test cases MUST have fields Nov 25, 2016
@xiekeyang xiekeyang changed the title fix bug: config test cases MUST have fields fix bug: config test cases MUST include all required fields Nov 29, 2016
@vbatts
Copy link
Member

vbatts commented Nov 30, 2016

LGTM

Approved with PullApprove

1 similar comment
@stevvooe
Copy link
Contributor

stevvooe commented Nov 30, 2016

LGTM

Approved with PullApprove

@stevvooe stevvooe merged commit 621a4e1 into opencontainers:master Nov 30, 2016
@xiekeyang xiekeyang deleted the config-test branch February 20, 2017 10:46
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.

4 participants