Skip to content

Limit TestEventsLimit to 4 concurrent containers on Windows#28968

Merged
LK4D4 merged 1 commit intomoby:masterfrom
darstahl:TestEventsLimit
Jan 30, 2017
Merged

Limit TestEventsLimit to 4 concurrent containers on Windows#28968
LK4D4 merged 1 commit intomoby:masterfrom
darstahl:TestEventsLimit

Conversation

@darstahl
Copy link
Copy Markdown
Contributor

@darstahl darstahl commented Nov 30, 2016 •
edited
Loading

This hopefully fixes the latest TestEventsLimit failures on windowsRS1.

fixes #28873

Signed-off-by: Darren Stahl darst@microsoft.com

@tonistiigi
Copy link
Copy Markdown
Member

tonistiigi commented Dec 8, 2016

That seems like a strange fix. If it is a platform bug then can we add a more specific comment about what command fails with what message and if possible a way to check the status of that bug.

@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented Jan 19, 2017

ping @darrenstahlmsft

LK4D4
LK4D4 suggested changes Jan 27, 2017
View reviewed changes
integration-cli/docker_cli_events_test.go Outdated
Copy link
Copy Markdown
Contributor

@LK4D4 LK4D4 Jan 27, 2017

Choose a reason for hiding this comment

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

@darrenstahlmsft this comment looks not true, we still limiting to 8 everywhere apart from Windows.

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented Jan 27, 2017

@darrenstahlmsft Is there a particular bug which will be fixed in windows? Or is it by design?

@darstahl
Limit TestEventsLimit to 4 concurrent containers on Windows
2ec808e 
Signed-off-by: Darren Stahl <darst@microsoft.com>
@darstahl
Copy link
Copy Markdown
Contributor Author

darstahl commented Jan 30, 2017

@LK4D4 Fixed so Windows is the only platform getting concurrency limits in this test.

@tonistiigi Is the new comment enough for you? Unfortunately there is no way to check the status of internal Windows bugs :( I could add a tracking number for Microsoft employees if that helps, but I don't think that will be useful

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented Jan 30, 2017

LGTM

1 similar comment
@tonistiigi
Copy link
Copy Markdown
Member

tonistiigi commented Jan 30, 2017

LGTM

@LK4D4 LK4D4 merged commit cdc79c5 into moby:master Jan 30, 2017
@GordonTheTurtle GordonTheTurtle added this to the 1.14.0 milestone Jan 30, 2017
@darstahl darstahl deleted the TestEventsLimit branch May 10, 2017 00:44
@thaJeztah thaJeztah mentioned this pull request Dec 20, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@LK4D4 LK4D4 LK4D4 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/testing status/4-merge

Projects

None yet

Milestone

17.04.0

Development

Successfully merging this pull request may close these issues.

Flaky test: TestEventsLimit (on windowsRS1)

7 participants

@darstahl @tonistiigi @AkihiroSuda @LK4D4 @vdemeester @lowenna @GordonTheTurtle