-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Put all docker compose files into environment before starting #755
Conversation
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.
Ah - good catch, and sorry if this caused you some problems. This looks good to me.
I think I'd like to have a test class to demonstrate that the problem is resolved. Is that something you could perhaps add?
I can try, I'm not good with grade, though. Which command should I run to
compile and run tests?
…On Fri, Jun 15, 2018, 18:34 Richard North ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Ah - good catch, and sorry if this caused you some problems. This looks
good to me.
I think I'd like to have a test class to demonstrate that the problem is
resolved. Is that something you could perhaps add?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#755 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABT6A2_3YaaQXplsYOEdD21FDiACCsMrks5t89P5gaJpZM4UpTTq>
.
|
Thanks - if you run The tests take quite a while to run, so I'd recommend just running your own test class in your IDE if you can while developing it. Thanks again. |
I think this test will require Also, Intellij fails compilation, and from command-line I cannot run single test for some reason:
|
Ok, with new test, but without the fix:
With fix test passes |
BTW, could you please release this as a patch release, if this is not too much trouble? |
Thanks @zbstof, I'll have a look now! |
I'm just going to push a slight change if that's OK; I've combined three similar test classes into one class that has parameterized tests. This was something that needed to be done before your change, it's just a bit more necessary now. |
Thanks for this @zbstof - I've merged into master |
Currently, only first docker-compose entry is put into environment:
will fail, when started, to the effect that properties from second docker-compose weren't applied