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

chore: use the same localstack version in all tests #259

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

moadibfr
Copy link
Contributor

No description provided.

@BeforeAll
static void startLocalstack() {
localstack = new LocalStackContainer(DockerImageName.parse(LOCALSTACK_VERSION))
.withServices(LocalStackContainer.Service.S3);
Copy link
Member

Choose a reason for hiding this comment

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

The service to start is different on each test, you can pass it via an abstract protected method for eg.

localstack = new LocalStackContainer(DockerImageName.parse(LOCALSTACK_VERSION))
.withServices(LocalStackContainer.Service.S3);
// some tests use a real flow with hardcoded configuration, so we have to fix the binding port
localstack.setPortBindings(java.util.List.of("4566:4566"));
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should'nt allow to configure the local stack before starting it (in an abstract protected method) as not all tst need this port binding and some test may need at some point other thing

Copy link
Member

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

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

LGTM

@moadibfr moadibfr marked this pull request as ready for review September 26, 2023 10:50
@moadibfr moadibfr merged commit 46a252b into master Sep 26, 2023
1 check passed
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.

2 participants