-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add support for multiple platform options in image load and save #6126
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,7 +104,7 @@ func TestNewLoadCommandSuccess(t *testing.T) { | |
| }, | ||
| }, | ||
| { | ||
| name: "with platform", | ||
| name: "with-single-platform", | ||
| args: []string{"--platform", "linux/amd64"}, | ||
| imageLoadFunc: func(input io.Reader, options ...client.ImageLoadOption) (image.LoadResponse, error) { | ||
| // FIXME(thaJeztah): need to find appropriate way to test the result of "ImageHistoryWithPlatform" being applied | ||
|
|
@@ -113,6 +113,22 @@ func TestNewLoadCommandSuccess(t *testing.T) { | |
| return image.LoadResponse{Body: io.NopCloser(strings.NewReader("Success"))}, nil | ||
| }, | ||
| }, | ||
| { | ||
| name: "with-comma-separated-platforms", | ||
| args: []string{"--platform", "linux/amd64,linux/arm64/v8,linux/riscv64"}, | ||
| imageLoadFunc: func(input io.Reader, options ...client.ImageLoadOption) (image.LoadResponse, error) { | ||
| assert.Check(t, len(options) > 0) // can be 1 or two depending on whether a terminal is attached :/ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm I'm not sure I understand this one. How does terminal affect parsing here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was simply copying a comment in the test right above this line (i.e.,
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah right.. We should apply these opts to a temporary var and then check the resulting opts struct, but we can't do that because these types are unexported.. I guess the only way to test it in this situation is by checking HTTP request at the HTTP client, but that would need rewriting the test a bit. Although, I think the best solution would be to have a debug helper on the client side that would allow introspecting for testing purposes...
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should do that in a follow up though |
||
| return image.LoadResponse{Body: io.NopCloser(strings.NewReader("Success"))}, nil | ||
| }, | ||
| }, | ||
| { | ||
| name: "with-multiple-platform-options", | ||
| args: []string{"--platform", "linux/amd64", "--platform", "linux/arm64/v8", "--platform", "linux/riscv64"}, | ||
| imageLoadFunc: func(input io.Reader, options ...client.ImageLoadOption) (image.LoadResponse, error) { | ||
| assert.Check(t, len(options) > 0) // can be 1 or two depending on whether a terminal is attached :/ | ||
| return image.LoadResponse{Body: io.NopCloser(strings.NewReader("Success"))}, nil | ||
| }, | ||
| }, | ||
| } | ||
| for _, tc := range testCases { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Success |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Success |
Uh oh!
There was an error while loading. Please reload this page.