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

Modify skopeo tests #399

Merged
merged 1 commit into from
Aug 31, 2017
Merged

Modify skopeo tests #399

merged 1 commit into from
Aug 31, 2017

Conversation

umohnani8
Copy link
Member

The oci name changes in containers/image containers/image#318 caused the skopeo test to fail

Signed-off-by: umohnani8 umohnani@redhat.com

@umohnani8
Copy link
Member Author

@mtrmac PTAL

Copy link
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

ACK modulo a minor point


// docker v2s2 -> OCI image layout without image name
ociDest = "busybox-latest-noimage"
assertSkopeoFails(c, ".*Error writing manifest: cannot save image with empty image.ref.name.*", "copy", "docker://busybox:latest", "oci:"+ociDest)
Copy link
Contributor

Choose a reason for hiding this comment

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

Testing this scenario might detect problems earlier if it were a unit test in the oci transport; testing that here doesn’t hurt, sure; but shouldn’t the busybox-latest-noimage directory also be cleaned up? The error is reported in PutManifest, which means PutBlob has already written files into the directory AFAICS.

(Even the old code should have used ioutil.TempDir instead of writing to the current directory, but that’s of course independent of this PR and not a blocker for it.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

LGTM

vendor.conf Outdated
@@ -1,5 +1,6 @@
github.com/urfave/cli v1.17.0
github.com/containers/image master
# github.com/containers/image master
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: please drop this comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

whoops, my bad. Fixed.

@umohnani8 umohnani8 changed the title [DO NOT MERGE] Modify skopeo tests Modify skopeo tests Aug 31, 2017
@mtrmac
Copy link
Contributor

mtrmac commented Aug 31, 2017

@umohnani8 Please update the expected error message:

... value string = "time=\"2017-08-31T15:43:50Z\" level=fatal msg=\"Error initializing destination oci:busybox-latest-noimage:: cannot save image with empty image.ref.name\" \n"
... regex string = "(?s).*Error writing manifest: cannot save image with empty image.ref.name.*"

The oci name changes in containers/image caused the skopeo test to fail

Signed-off-by: umohnani8 <umohnani@redhat.com>
@mtrmac
Copy link
Contributor

mtrmac commented Aug 31, 2017

Thanks!

@mtrmac mtrmac merged commit a9e8c58 into containers:master Aug 31, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants