-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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
Implement service integration tests #24133
Conversation
2ff9685
to
3fcee97
Compare
ping @vdemeester: there you go! 😄 |
3fcee97
to
9d00e11
Compare
@@ -19,6 +20,7 @@ type SwarmDaemon struct { | |||
swarm.Info | |||
port int | |||
listenAddr string | |||
suite *DockerSwarmSuite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this being used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hHuzzah, I'm not! I was but I guess I ended up not needing it anymore.
9d00e11
to
8a9b659
Compare
id := strings.TrimSpace(out) | ||
|
||
var tasks []swarm.Task | ||
wait(c, 30*time.Second, func() bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
30 -> defaultReconciliationTimeout
. Also I think waitAndAssert
function should be usable here and provides better output.
eebeb57
to
e4f83a7
Compare
Updated |
@cpuguy83 Windows tests are failing because of missing build tags. |
e4f83a7
to
86c5bc1
Compare
This is done in a hacky way as currently there is no better way. Uses known implementation details about how tasks are scheduled to be able to operate on the underlying container. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
86c5bc1
to
e5ec575
Compare
This is now 100% working and ready for review. |
LGTM |
LGTM |
This is done in a hacky way as currently there is no better way (that I know of).
Uses known implementation details about how tasks are scheduled to be
able to operate on the underlying container.
The one test I added is actually exposing a bug (fixed by #23947). Once this is merged we should add more tests to ensure that resulting containers match expectations from a service spec.
I've intentionally put these tests and related helpers into files with
_hack_
in the name so once there is a better way, we can clean these up easily.I would be worried about releasing 1.12 without more tests like this.