test: migrate test api network get defaults and filter by mdaffad · Pull Request #50529 · moby/moby

mdaffad · 2025-07-27T15:06:43Z

- What I did

Migrated the TestAPINetworkGetDefaults and TestAPINetworkFilter to integration tests in reference to the integration-cli migration epic #50159

- How I did it

Using test on integration-cli/docker_api_network_test.go

- How to verify it

Run integration test

thaJeztah · 2025-07-27T16:29:58Z

Thx! Didn't review yet, but see there's some linting failures (looks like easy fix);

integration/network/network_test.go:13:2: ST1019: package "github.com/moby/moby/api/types/network" is being imported more than once (staticcheck)
	"github.com/moby/moby/api/types/network"
	^
integration/network/network_test.go:14:2: ST1019(related information): other import of "github.com/moby/moby/api/types/network" (staticcheck)
	networktypes "github.com/moby/moby/api/types/network"
	^
integration-cli/docker_api_network_test.go:217:6: func getNetworkIDByName is unused (unused)
func getNetworkIDByName(t *testing.T, name string) string {
     ^

thaJeztah · 2025-07-27T16:34:12Z

integration/network/network_test.go

 }
+
+func TestAPINetworkGetDefaults(t *testing.T) {
+	skip.If(t, runtime.GOOS == "windows")


Looks like this is a test specific for the bridge network, so will never be "un skipped" - so probably makes sense to put it in the _linux_test.go file

PERHAPS (but didn't look closely) even in https://github.com/moby/moby/tree/master/integration/network/bridge/

if it's for the bridge driver

For this, i think if we solve the case for network name for windows platform, it will be also fixable. "bridge" for this case is the defaults created network name.

If that is not the case, i will move to your _linux_test.go options.

Updated, give default network name for windows use case

thaJeztah · 2025-07-27T16:35:22Z

integration/network/network_test.go

+	ctx := setupTest(t)
+	apiClient := testEnv.APIClient()
+
+	networkName := "bridge"


This may be fixable for windows if we make the name of the network depend on platform

Updated, give "nat" network name for windows

akerouanton

Two nits, otherwise LGTM.

integration/network/network_test.go

akerouanton

LGTM

robmry

Hi @mdaffad - thank you for this, and sorry it's taken us so long to approve the PR.

Unfortunately, because we were slow, although there's no merge conflict it needs a rebase and an update. There's been a lot of refactoring of API types, but this should do the trick ...

diff --git a/integration/network/network_test.go b/integration/network/network_test.go
index 03365d21bd..10e5177a34 100644
--- a/integration/network/network_test.go
+++ b/integration/network/network_test.go
@@ -9,10 +9,10 @@ import (

        "github.com/moby/moby/api/types/filters"
        networktypes "github.com/moby/moby/api/types/network"
+       "github.com/moby/moby/client"
        "github.com/moby/moby/v2/internal/testutil"
        "github.com/moby/moby/v2/internal/testutil/request"
        "gotest.tools/v3/assert"
-
        is "gotest.tools/v3/assert/cmp"
 )

@@ -131,7 +131,7 @@ func TestAPINetworkFilter(t *testing.T) {
        ctx := setupTest(t)
        apiClient := testEnv.APIClient()

-       networks, err := apiClient.NetworkList(ctx, networktypes.ListOptions{
+       networks, err := apiClient.NetworkList(ctx, client.NetworkListOptions{
                Filters: filters.NewArgs(filters.Arg("name", networkName)),
        })

Please squash changes into a single commit and force-push to keep the git history clean - or I can make the update if you prefer, just let me know.

thaJeztah · 2025-09-19T12:33:31Z

integration/network/network_test.go

+	apiClient := testEnv.APIClient()
+
+	defaults := []string{"bridge", "host", "none"}
+	if runtime.GOOS == "windows" {


Probably more correct would be to check the daemon's platform. We don't currently do so, but these tests could be run from a Linux environment testing a Windows daemon;

if testEnv.DaemonInfo.OSType == "windows" {

thaJeztah · 2025-09-19T12:34:10Z

integration/network/network_test.go

+
+func TestAPINetworkFilter(t *testing.T) {
+	networkName := "bridge"
+	if runtime.GOOS == "windows" {


Same here;

if testEnv.DaemonInfo.OSType == "windows" {

Signed-off-by: Muhammad Daffa Dinaya <muhammaddaffadinaya@gmail.com>

robmry

LGTM - thank you.

The test failures are unrelated, GitHub having a bad day I think.

thaJeztah

LGTM

mdaffad force-pushed the 50159-migrate-test-api-network-defaults-and-filter branch 2 times, most recently from 07124f3 to 1841885 Compare July 27, 2025 15:09

thaJeztah added status/2-code-review area/networking Networking area/testing kind/refactor PR's that refactor, or clean-up code labels Jul 27, 2025

thaJeztah reviewed Jul 27, 2025

View reviewed changes

mdaffad force-pushed the 50159-migrate-test-api-network-defaults-and-filter branch 4 times, most recently from a3cd9d6 to 96afa4e Compare August 5, 2025 02:42

akerouanton reviewed Aug 5, 2025

View reviewed changes

integration/network/network_test.go Outdated Show resolved Hide resolved

integration/network/network_test.go Outdated Show resolved Hide resolved

mdaffad force-pushed the 50159-migrate-test-api-network-defaults-and-filter branch from 96afa4e to 2d942ee Compare August 5, 2025 13:51

akerouanton approved these changes Aug 6, 2025

View reviewed changes

mdaffad requested a review from thaJeztah August 25, 2025 15:10

akerouanton requested a review from robmry September 19, 2025 06:54

robmry requested changes Sep 19, 2025

View reviewed changes

thaJeztah reviewed Sep 19, 2025

View reviewed changes

thaJeztah mentioned this pull request Sep 19, 2025

integration: migrate TestAPICreateDeletePredefinedNetworks from integration-cli #51007

Open

test: migrate test api network get defaults and filter

87d1da5

Signed-off-by: Muhammad Daffa Dinaya <muhammaddaffadinaya@gmail.com>

mdaffad force-pushed the 50159-migrate-test-api-network-defaults-and-filter branch from 2d942ee to 87d1da5 Compare September 20, 2025 06:02

mdaffad requested review from robmry and thaJeztah September 20, 2025 06:02

robmry approved these changes Sep 22, 2025

View reviewed changes

thaJeztah approved these changes Sep 22, 2025

View reviewed changes

thaJeztah merged commit 69d5112 into moby:master Sep 22, 2025
401 of 412 checks passed

thaJeztah added this to the 29.0.0 milestone Oct 6, 2025

robmry mentioned this pull request Nov 19, 2025

integration-cli migration #50159

Open

Conversation

mdaffad commented Jul 27, 2025

Uh oh!

thaJeztah commented Jul 27, 2025

Uh oh!

thaJeztah Jul 27, 2025

Choose a reason for hiding this comment

Uh oh!

mdaffad Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

mdaffad Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jul 27, 2025

Choose a reason for hiding this comment

Uh oh!

mdaffad Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

akerouanton left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

akerouanton left a comment

Choose a reason for hiding this comment

Uh oh!

robmry left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

thaJeztah Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

robmry left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants