Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

integ-cli: Skip new tests requiring same-host daemon #10995

Merged
merged 1 commit into from
Feb 25, 2015
Merged

integ-cli: Skip new tests requiring same-host daemon #10995

merged 1 commit into from
Feb 25, 2015

Conversation

ahmetb
Copy link
Contributor

@ahmetb ahmetb commented Feb 24, 2015

This skips the following tests:

  • TestEventsFilters
  • TestExecAfterContainerRestart
  • TestRunExecDir
  • TestRunMutableNetworkFiles

as the tests require starting a daemon on the same host.

Signed-off-by: Ahmet Alp Balkan ahmetalpbalkan@gmail.com
cc: @unclejack @jfrazelle @LK4D4

@jessfraz
Copy link
Contributor

sorry this was my fault LGTM

@jessfraz
Copy link
Contributor

ok new idea if you want in TestEventsFilters we can remove:

events = events[:len(events)-1]
 if len(events) != 2 {
    t.Fatalf("Expected 2 events, got %d: %v", len(events), events)  
 }          

and instead iterate each line of events and make sure they are die, then we dont need a new daemon, up to you

@ahmetb
Copy link
Contributor Author

ahmetb commented Feb 25, 2015

@jfrazelle there's certain value in this test for the CLI. I'm okay waiting if you want to send a PR to fix the test. I can amend to skip only TestExecAfterContainerRestart in this commit.

@jessfraz
Copy link
Contributor

fixed #10998

This skips tests:
- `TestExecAfterContainerRestart`

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
@ahmetb
Copy link
Contributor Author

ahmetb commented Feb 25, 2015

amended commit.

@jessfraz
Copy link
Contributor

LGTM

1 similar comment
@cpuguy83
Copy link
Member

LGTM

jessfraz pushed a commit that referenced this pull request Feb 25, 2015
…skips-2

integ-cli: Skip new tests requiring same-host daemon
@jessfraz jessfraz merged commit 1060a53 into moby:master Feb 25, 2015
@ahmetb
Copy link
Contributor Author

ahmetb commented Feb 25, 2015

ooh I was gonna skip more tests in the last minute, I guess I'll just open a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants