Remove duplicate TestServiceUpdatePort by thaJeztah · Pull Request #36502 · moby/moby

thaJeztah · 2018-03-06T15:19:39Z

The TestAPIServiceUpdatePort test performs exactly the same steps.

Here's one;

moby/integration-cli/docker_cli_service_update_test.go

Lines 14 to 47 in 5a48410

    
           func (s *DockerSwarmSuite) TestServiceUpdatePort(c *check.C) { 
        
           	d := s.AddDaemon(c, true, true) 
        
           	serviceName := "TestServiceUpdatePort" 
        
           	serviceArgs := append([]string{"service", "create", "--detach", "--no-resolve-image", "--name", serviceName, "-p", "8080:8081", defaultSleepImage}, sleepCommandForDaemonPlatform()...) 
        
           	// Create a service with a port mapping of 8080:8081. 
        
           	out, err := d.Cmd(serviceArgs...) 
        
           	c.Assert(err, checker.IsNil) 
        
           	waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1) 
        
           	// Update the service: changed the port mapping from 8080:8081 to 8082:8083. 
        
           	_, err = d.Cmd("service", "update", "--detach", "--publish-add", "8082:8083", "--publish-rm", "8081", serviceName) 
        
           	c.Assert(err, checker.IsNil) 
        
           	// Inspect the service and verify port mapping 
        
           	expected := []swarm.PortConfig{ 
        
           		{ 
        
           			Protocol:      "tcp", 
        
           			PublishedPort: 8082, 
        
           			TargetPort:    8083, 
        
           			PublishMode:   "ingress", 
        
           		}, 
        
           	} 
        
           	out, err = d.Cmd("service", "inspect", "--format", "{{ json .Spec.EndpointSpec.Ports }}", serviceName) 
        
           	c.Assert(err, checker.IsNil) 
        
           	var portConfig []swarm.PortConfig 
        
           	if err := json.Unmarshal([]byte(out), &portConfig); err != nil { 
        
           		c.Fatalf("invalid JSON in inspect result: %v (%s)", err, out) 
        
           	} 
        
           	c.Assert(portConfig, checker.DeepEquals, expected) 
        
           }

And the other;

moby/integration-cli/docker_api_swarm_service_test.go

Lines 32 to 51 in 5a48410

    
           func (s *DockerSwarmSuite) TestAPIServiceUpdatePort(c *check.C) { 
        
           	d := s.AddDaemon(c, true, true) 
        
           	// Create a service with a port mapping of 8080:8081. 
        
           	portConfig := []swarm.PortConfig{{TargetPort: 8081, PublishedPort: 8080}} 
        
           	serviceID := d.CreateService(c, simpleTestService, setInstances(1), setPortConfig(portConfig)) 
        
           	waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1) 
        
           	// Update the service: changed the port mapping from 8080:8081 to 8082:8083. 
        
           	updatedPortConfig := []swarm.PortConfig{{TargetPort: 8083, PublishedPort: 8082}} 
        
           	remoteService := d.GetService(c, serviceID) 
        
           	d.UpdateService(c, remoteService, setPortConfig(updatedPortConfig)) 
        
           	// Inspect the service and verify port mapping. 
        
           	updatedService := d.GetService(c, serviceID) 
        
           	c.Assert(updatedService.Spec.EndpointSpec, check.NotNil) 
        
           	c.Assert(len(updatedService.Spec.EndpointSpec.Ports), check.Equals, 1) 
        
           	c.Assert(updatedService.Spec.EndpointSpec.Ports[0].TargetPort, check.Equals, uint32(8083)) 
        
           	c.Assert(updatedService.Spec.EndpointSpec.Ports[0].PublishedPort, check.Equals, uint32(8082)) 
        
           }

The TestAPIServiceUpdatePort test performs exactly the same steps. Signed-off-by: Sebastiaan van Stijn <[email protected]>

vdemeester

LGTM 🐯

dnephin

LGTM

thaJeztah · 2018-03-06T16:39:22Z

hm... https://jenkins.dockerproject.org/job/Docker-PRs-experimental/39654/console

15:31:45 === RUN   TestServiceWithPredefinedNetwork
15:31:56 --- FAIL: TestServiceWithPredefinedNetwork (11.33s)
15:31:56 	daemon.go:283: [d016408df2853] waiting for daemon to start
15:31:56 	daemon.go:315: [d016408df2853] daemon started
15:31:56 	service_test.go:52: timeout hit after 10s: task count at 1 waiting for 0
15:31:56 	daemon.go:273: [d016408df2853] exiting daemon
15:31:56 FAIL

Odd, that test was added in #36316, but trying to understand how waiting for 0 can occur

thaJeztah · 2018-03-06T16:47:05Z

oh! misreading, was looking at service count, not task-count (thanks @vdemeester), so that test is failing at

moby/integration/network/service_test.go

Line 52 in 0b0af85

poll.WaitOn(t, noTasks(client), pollSettings)

yongtang

LGTM

codecov · 2018-03-06T23:13:21Z

Codecov Report

❗ No coverage uploaded for pull request base (master@f0694e9). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #36502   +/-   ##
=========================================
  Coverage          ?   34.64%           
=========================================
  Files             ?      613           
  Lines             ?    45404           
  Branches          ?        0           
=========================================
  Hits              ?    15730           
  Misses            ?    27614           
  Partials          ?     2060

Remove duplicate TestServiceUpdatePort

36e1646

The TestAPIServiceUpdatePort test performs exactly the same steps. Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah added status/2-code-review area/testing labels Mar 6, 2018

thaJeztah requested review from dnephin and vdemeester as code owners March 6, 2018 15:19

GordonTheTurtle added the status/0-triage label Mar 6, 2018

thaJeztah mentioned this pull request Mar 6, 2018

Flaky tests: Service "port" tests #36501

Open

thaJeztah removed the status/0-triage label Mar 6, 2018

vdemeester approved these changes Mar 6, 2018

View reviewed changes

dnephin approved these changes Mar 6, 2018

View reviewed changes

yongtang approved these changes Mar 6, 2018

View reviewed changes

yongtang added status/4-merge rebuild/experimental and removed status/2-code-review labels Mar 6, 2018

GordonTheTurtle removed the rebuild/experimental label Mar 6, 2018

yongtang added the rebuild/windowsRS1 label Mar 6, 2018

GordonTheTurtle removed the rebuild/windowsRS1 label Mar 6, 2018

yongtang added the rebuild/experimental label Mar 6, 2018

GordonTheTurtle removed the rebuild/experimental label Mar 6, 2018

yongtang added the rebuild/z label Mar 6, 2018

GordonTheTurtle removed the rebuild/z label Mar 6, 2018

yongtang merged commit f3a3778 into moby:master Mar 6, 2018

thaJeztah deleted the remove-duplicate-test branch March 6, 2018 23:24

thaJeztah mentioned this pull request Apr 17, 2018

[18.03] backport updates to tests, and fix empty LogPath with non-blocking logging mode docker-archive/docker-ce#512

Merged

thaJeztah mentioned this pull request Aug 31, 2018

[epic] flaky tests #37306

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove duplicate TestServiceUpdatePort#36502

Remove duplicate TestServiceUpdatePort#36502
yongtang merged 1 commit intomoby:masterfrom
thaJeztah:remove-duplicate-test

thaJeztah commented Mar 6, 2018

Uh oh!

vdemeester left a comment

Uh oh!

dnephin left a comment

Uh oh!

thaJeztah commented Mar 6, 2018

Uh oh!

thaJeztah commented Mar 6, 2018

Uh oh!

yongtang left a comment

Uh oh!

codecov Bot commented Mar 6, 2018 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	func (s DockerSwarmSuite) TestServiceUpdatePort(c check.C) {
	d := s.AddDaemon(c, true, true)

	serviceName := "TestServiceUpdatePort"
	serviceArgs := append([]string{"service", "create", "--detach", "--no-resolve-image", "--name", serviceName, "-p", "8080:8081", defaultSleepImage}, sleepCommandForDaemonPlatform()...)

	// Create a service with a port mapping of 8080:8081.
	out, err := d.Cmd(serviceArgs...)
	c.Assert(err, checker.IsNil)
	waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1)

	// Update the service: changed the port mapping from 8080:8081 to 8082:8083.
	_, err = d.Cmd("service", "update", "--detach", "--publish-add", "8082:8083", "--publish-rm", "8081", serviceName)
	c.Assert(err, checker.IsNil)

	// Inspect the service and verify port mapping
	expected := []swarm.PortConfig{
	{
	Protocol: "tcp",
	PublishedPort: 8082,
	TargetPort: 8083,
	PublishMode: "ingress",
	},
	}

	out, err = d.Cmd("service", "inspect", "--format", "{{ json .Spec.EndpointSpec.Ports }}", serviceName)
	c.Assert(err, checker.IsNil)

	var portConfig []swarm.PortConfig
	if err := json.Unmarshal([]byte(out), &portConfig); err != nil {
	c.Fatalf("invalid JSON in inspect result: %v (%s)", err, out)
	}
	c.Assert(portConfig, checker.DeepEquals, expected)
	}

	func (s DockerSwarmSuite) TestAPIServiceUpdatePort(c check.C) {
	d := s.AddDaemon(c, true, true)

	// Create a service with a port mapping of 8080:8081.
	portConfig := []swarm.PortConfig{{TargetPort: 8081, PublishedPort: 8080}}
	serviceID := d.CreateService(c, simpleTestService, setInstances(1), setPortConfig(portConfig))
	waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1)

	// Update the service: changed the port mapping from 8080:8081 to 8082:8083.
	updatedPortConfig := []swarm.PortConfig{{TargetPort: 8083, PublishedPort: 8082}}
	remoteService := d.GetService(c, serviceID)
	d.UpdateService(c, remoteService, setPortConfig(updatedPortConfig))

	// Inspect the service and verify port mapping.
	updatedService := d.GetService(c, serviceID)
	c.Assert(updatedService.Spec.EndpointSpec, check.NotNil)
	c.Assert(len(updatedService.Spec.EndpointSpec.Ports), check.Equals, 1)
	c.Assert(updatedService.Spec.EndpointSpec.Ports[0].TargetPort, check.Equals, uint32(8083))
	c.Assert(updatedService.Spec.EndpointSpec.Ports[0].PublishedPort, check.Equals, uint32(8082))
	}

Conversation

thaJeztah commented Mar 6, 2018

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

dnephin left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Mar 6, 2018

Uh oh!

thaJeztah commented Mar 6, 2018

Uh oh!

yongtang left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Mar 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Mar 6, 2018 •

edited

Loading