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 support for https://docs.docker.com/compose/extends/ #214

Closed
wants to merge 5 commits into from

Conversation

sourcecase
Copy link
Contributor

It is possible to pass more then one docker-compose file to the docker-compose comand as decribed here https://docs.docker.com/compose/extends/

To make this feature available in testcontainers a list of docker-compose files ahs to be passed to DockerComposeContainer.

The files have to be concatenated in the COMPOSE_FILE env variable as described here https://docs.docker.com/compose/reference/envvars/

@rnorth
Copy link
Member

rnorth commented Aug 25, 2016

This is a great idea - nice! I'll review tonight.

addEnv("COMPOSE_FILE", pwd + "/" + composeFile.getAbsoluteFile().getName());
File dockerComposeMainFile = composeFiles.get(0);
final String pwd = dockerComposeMainFile.getAbsoluteFile().getParentFile().getAbsolutePath();
List<String> absoluteDockerComposeFiles = composeFiles.stream().map(file -> pwd + "/" + file.getAbsoluteFile().getName()).collect(Collectors.toList());
Copy link
Member

Choose a reason for hiding this comment

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

Please could you use File.separator rather than "/", just in case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The benefit of "/" is that it works both on windows and unix like systems. This why used it instead of File.separator

@rnorth
Copy link
Member

rnorth commented Aug 26, 2016

Please could we also have some tests to ensure this is working?

@sourcecase
Copy link
Contributor Author

Sure I will add some tests and will trigger again.

@rnorth
Copy link
Member

rnorth commented Sep 21, 2016

@sourcecase did you have a chance to create some tests for this yet?

@sourcecase
Copy link
Contributor Author

Hi Richard,
not yet. But thanks for the reminder! I plan to add some this weekend.
Regards
Chris

@sourcecase
Copy link
Contributor Author

I created a new pull request for the same feature from a different branch including tests: #227

@sourcecase sourcecase closed this Sep 23, 2016
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.

None yet

2 participants