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

manifest test case should involve required fields #446

Merged
merged 1 commit into from
Nov 17, 2016
Merged

manifest test case should involve required fields #446

merged 1 commit into from
Nov 17, 2016

Conversation

xiekeyang
Copy link
Contributor

@xiekeyang xiekeyang commented Nov 5, 2016

This test case want to return failure only when invalid media type.
However, it lack the required fields, and will return failure whatever mediaType is.
So, it should be completed the accurate required fields, to make sure it to be our expectation.

Signed-off-by: xiekeyang [email protected]

@wking
Copy link
Contributor

wking commented Nov 5, 2016

On Sat, Nov 05, 2016 at 04:40:37AM -0700, xiekeyang wrote:

This test case want to return failure only when invalid media type.
However, it lack the required fields, and will return failure whatever mediaType is.
So, it should be completed the accurate required fields, to make sure the test aim.

fb409fd looks good to me.

Now it fails with:

mediaType: Does not match pattern '^[a-z]+/[0-9a-zA-Z.+]+$'

So a similar test with mediaType set to text/png would pass, although
the manifest would obviously still be invalid. The fix for that is
in #404.

@xiekeyang
Copy link
Contributor Author

@stevvooe @vbatts This PR is what I think to be ready, and PTAL, thanks.

@philips
Copy link
Contributor

philips commented Nov 17, 2016

LGTM

Approved with PullApprove

@jonboulle
Copy link
Contributor

jonboulle commented Nov 17, 2016

LGTM

Approved with PullApprove

@jonboulle jonboulle merged commit f40938c into opencontainers:master Nov 17, 2016
@xiekeyang xiekeyang deleted the manifest-unit-test branch February 20, 2017 10:57
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