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

Add an integration/internal/container helper package #36266

Merged
merged 1 commit into from
Feb 10, 2018

Conversation

vdemeester
Copy link
Member

@vdemeester vdemeester commented Feb 9, 2018

On top of #36265

To help creating/running/… containers using the client for test integration.
This should make test more readable and reduce duplication a bit.

Usage example

// Create a default container named foo
id1 := container.Create(t, ctx, client, container.WithName("foo"))
// Run a default container with a custom command
id2 := container.Run(t, ctx, client, container.WithCmd("echo", "hello world"))

This replace runSimpleContainer and createSimpleContainer 👼

Signed-off-by: Vincent Demeester vincent@sbr.pm

Copy link
Member

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM

}

func Create(t *testing.T, ctx context.Context, client client.APIClient, ops ...func(*TestContainerConfig)) string {
config := &TestContainerConfig{
Copy link
Member

Choose a reason for hiding this comment

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

Should we t.Helper() in these so the error gets reported from the caller?

)

type TestContainerConfig struct {
Name string
Copy link
Member

Choose a reason for hiding this comment

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

I wish we had this struct (and ops) for the actual client package! I guess we can look at doing that later

Copy link
Member Author

Choose a reason for hiding this comment

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

That's definitely sthg we should do 👼 (It's now on my todo 😛)

@vdemeester vdemeester force-pushed the integration-container-helper branch from 8278c63 to c4054ff Compare February 10, 2018 08:19
@vdemeester vdemeester changed the title wip: Add an integration/internal/container helper package Add an integration/internal/container helper package Feb 10, 2018
@vdemeester vdemeester force-pushed the integration-container-helper branch from c4054ff to 0958efb Compare February 10, 2018 16:24
To help creating/running/… containers using the client for test integration.
This should make test more readable and reduce duplication a bit.

Usage example

```
// Create a default container named foo
id1 := container.Create(t, ctx, client, container.WithName("foo"))
// Run a default container with a custom command
id2 := container.Run(t, ctx, client, container.WithCmd("echo", "hello world"))
```

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
@vdemeester vdemeester force-pushed the integration-container-helper branch from 0958efb to 0bb7d42 Compare February 10, 2018 16:29
Copy link
Member

@yongtang yongtang left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

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

Successfully merging this pull request may close these issues.

4 participants