integration-cli: migrate TestAPIStatsContainerNotFound to integration tests#51369
integration-cli: migrate TestAPIStatsContainerNotFound to integration tests#51369robmry merged 1 commit intomoby:masterfrom
Conversation
robmry
left a comment
There was a problem hiding this comment.
Hi @dohrisalim - thank you for taking a look at this!
It needs a rebase.
Could you remove the PR link from the commit comment, as GitHub tends to spam PRs when commits are merged.
And, a couple of suggestions (please squash commits) ...
integration/container/stats_test.go
Outdated
| } | ||
|
|
||
| func TestStatsContainerNotFound(t *testing.T) { | ||
| skip.If(t, testEnv.DaemonInfo.OSType == "windows") |
There was a problem hiding this comment.
The original test was skipped on Windows, but I'm not sure it needs to be - could try removing this?
integration/container/stats_test.go
Outdated
| ctx := setupTest(t) | ||
| apiClient := testEnv.APIClient() | ||
|
|
||
| _, err := apiClient.ContainerStats(ctx, "no-such-container", client.ContainerStatsOptions{ |
There was a problem hiding this comment.
As there are two tests doing the same thing (just with different options) - it'd be best to use a test array, see TestAttach for an example. Then each test gets its own result, the second still runs if the first fails, and we can add more if we think of them.
tests := []struct {
name string
options client.ContainerStatsOptions
}{
...
}
|
Thanks for the fast reply @robmry, I'm on it! |
6501d7b to
b82c165
Compare
|
Hi @robmry , can you take a look at this PR at your convenience. I made the changes you requested. |
Thanks - looks good, but needs a rebase to fix the merge conflict before tests will run. |
… tests Migrate TestAPIStatsContainerNotFound from the deprecated integration-cli test suite to the modern integration test framework in integration/container. The test verifies that the container stats API returns a NotFound error for non-existent containers, testing both streaming and non-streaming modes. Changes made: - Migrated test to integration/container/stats_test.go using standard Go testing patterns - Refactored to use test array pattern for better test organization - Removed test from integration-cli/docker_api_stats_test.go - Removed unused imports from integration-cli file - Removed Windows skip as it may not be necessary Signed-off-by: Salim Dohri <dohri.salim@gmail.com>
b82c165 to
9b749d7
Compare
|
Sorry about that I thought I did the rebase, the pr should be mergeable now. |
|
Hey👋 @robmry could you take a look at this pr when you're free? |
robmry
left a comment
There was a problem hiding this comment.
LGTM - thank you @dohrisalim.
|
I'll get this merged ... test changes only, and the tests pass. |
Summary
Migrates
TestAPIStatsContainerNotFoundfrom the deprecatedintegration-clitest suite to the modernintegration/containerpackage following established migration patterns.Changes
TestStatsContainerNotFoundfunction inintegration/container/stats_test.goTestAPIStatsContainerNotFoundfunction fromintegration-cli/docker_api_stats_test.gocerrdefsandclient) from integration-cli fileTest Description
The test validates that the
ContainerStatsAPI correctly returnsNotFounderrors when requesting stats for non-existent containers, covering both:Stream: true)Stream: false)Migration Patterns Applied
check.v1framework to standardtestingpackagegotest.tools/v3/asserttestRequires(c, DaemonIsLinux)toskip.If(t, testEnv.DaemonInfo.OSType == "windows")testutil.GetContext(c)→setupTest(t)client.NewClientWithOpts(client.FromEnv)→testEnv.APIClient()Test Results
Related Issue
Related to #50159