Skip to content

migrate TestAPIImageImportBadSrc to integration test#51553

Merged
robmry merged 1 commit intomoby:masterfrom
2003Aditya:TestAPIImagesImportBadSrc
Jan 5, 2026
Merged

migrate TestAPIImageImportBadSrc to integration test#51553
robmry merged 1 commit intomoby:masterfrom
2003Aditya:TestAPIImagesImportBadSrc

Conversation

@2003Aditya
Copy link
Contributor

@2003Aditya 2003Aditya commented Nov 17, 2025

- What I did
Migrated the TestAPIImageImportBadSrc test from integration-cli/docker_api_image_test.go to the new integration test framework under integration/image/import_test.go.

@github-actions github-actions bot added area/testing area/images Image Distribution labels Nov 17, 2025
@2003Aditya 2003Aditya force-pushed the TestAPIImagesImportBadSrc branch 3 times, most recently from 7e7337b to 77d101c Compare November 18, 2025 07:18
Copy link
Contributor

@robmry robmry left a comment

Choose a reason for hiding this comment

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

Thank you @2003Aditya ...

Comment on lines +240 to +241
t.Logf("err = %#v", err)

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no need to log here - if the assertion below fails, it'll show the error anyway.

t.Run(tc.name, func(t *testing.T) {
_, err := apiClient.ImageImport(ctx,
client.ImageImportSource{
Source: nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: there's no need to initialise Source to nil.

@2003Aditya 2003Aditya force-pushed the TestAPIImagesImportBadSrc branch 2 times, most recently from 787f8fc to df5bb82 Compare November 19, 2025 16:43
}
}

func TestAPIImageImportBadSrc(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

One more comment, then I think we're good to merge ...

Speaking with @thaJeztah last week - he asks that we try to use a common prefix for tests within a file. I'm not sure how consistent we've been with that in the past, but it'll be TestImport... in this case.

Suggested change
func TestAPIImageImportBadSrc(t *testing.T) {
func TestImageImportBadSrc(t *testing.T) {

@2003Aditya 2003Aditya force-pushed the TestAPIImagesImportBadSrc branch from df5bb82 to e507a93 Compare November 28, 2025 11:59
@robmry
Copy link
Contributor

robmry commented Nov 28, 2025

Oh, there's a merge conflict now (and, nit - a blank line at the end of the test).

@2003Aditya 2003Aditya force-pushed the TestAPIImagesImportBadSrc branch from e507a93 to 2220ab6 Compare November 28, 2025 17:50
@robmry
Copy link
Contributor

robmry commented Dec 5, 2025

Sorry for the delay ... looks like the rebase went wrong though - TestAPIImagesSaveAndLoad is back (you removed it in #51559).

Signed-off-by: Aditya Mishra <mishraaditya675@gmail.com>
@2003Aditya 2003Aditya force-pushed the TestAPIImagesImportBadSrc branch from 2220ab6 to ae4c17f Compare December 25, 2025 19:11
Copy link
Contributor

@robmry robmry left a comment

Choose a reason for hiding this comment

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

A couple of nits - but they can be dealt with in a follow-up. Let's get this merged.

ctx := setupTest(t)
apiClient := testEnv.APIClient()

skip.If(t, testEnv.IsRootless, "rootless daemon cannot access the test's HTTP server in the host's netns")
Copy link
Contributor

Choose a reason for hiding this comment

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

The skip should be before the test setup.

assert.Check(t, tc.expectErr(err))
})
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Blank line.

@robmry robmry merged commit 9f92b72 into moby:master Jan 5, 2026
180 checks passed
@thaJeztah thaJeztah added this to the 29.2.0 milestone Feb 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/images Image Distribution area/testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants