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

Docker compose tests #167

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

HackingM
Copy link
Contributor

@HackingM HackingM commented Sep 4, 2019

Hi Erik,

This is the last of the PRs (for now).

I have some images which require other images to be built before they can be tested. An example of this is a wordpress image which needs a mysql image for any real tests work.

This PR adds an extra test step using docker-compose to enable such images to be tested.

I have had to re-work (and simplify) the dependency code in build.sh and dep-graph.sh. At some point in the future I would like to unify these and move them to core.sh if you would approve.

Let me know what you think and thanks again for writing such a great tool.

@edannenberg
Copy link
Owner

edannenberg commented Sep 5, 2019

Integration tests with docker-compose have been in the backlog for a while now, so thanks a lot for this one! Sadly I'm still a bit short on time but gonna reserve some this weekend to review/merge your PRs.

@edannenberg
Copy link
Owner

Had some time to play with this over the weekend but got sidetracked by the broken Kibana image (Kibana requires a valid elasticsearch connection on startup so this was the perfect test case). Looks good so far though! Another useful feature for this could be the ability to run commands from outside the container as part of the test, I'll give this some more thought the next days.

@HackingM
Copy link
Contributor Author

Another useful feature for this could be the ability to run commands from outside the container as part of the test

It already does run the test script from outside the container. Did you mean inside the container?

@edannenberg
Copy link
Owner

It already does run the test script from outside the container. Did you mean inside the container?

Oops, no, I was wrongly assuming it's run from inside the container. :) It's probably enough to have the build-test.sh for in-container tests. Hopefully some time this week to finish testing.

@HackingM
Copy link
Contributor Author

Since writing this code I have discovered that some of our images require a more complex testing procedure involving multiple setup, test and finish steps.

An example of this is a PostgreSQL image which should:

  1. create an empty set of databases if run against an empty volume.
  2. dump and upgrade the databases if run against a volume containing a previous version's data.
  3. run normally when run against a volume containing data for the current version.

I am thinking that a compose-tests directory containing the test scripts and yml files may be the way forward with each script defining setup(), test() and finish() functions as appropriate.

Also, my unit/integration testing has revealed some minor issues which it might be wise to address before merging this PR. The most significant of these is that circular dependencies can be much more easily declared now and neither build nor dep-graph handles this condition nicely. It would make sense to me if build detected circular dependencies and dep-graph could handle rendering them correctly so they can be easily visualised.

Let me know what you think and I'll work something up for you to look at regarding the above issues when I get a spare moment.

@edannenberg
Copy link
Owner

edannenberg commented Sep 25, 2019

Sounds good to me! I'm very short on time for another week or so, happy to help where I can though.

PS: The PR is a bit messed up currently due to the merge commit, try:

git reset HEAD~1
(check if everything is ok)
git push --force-with-lease

if you are still missing the commits from master you can rebase the branch and push force again:

(assuming master branch is up to date)
git rebase master

@HackingM
Copy link
Contributor Author

HackingM commented Oct 4, 2019

Thanks.

git rebase master

is exactly what I did. Then I foolishly decided to use git from within Eclipse to push the changes (because I was having a conversation with someone). I wasn't really paying attention so when it asked if I wanted to do something which it claimed I needed to do I just agreed and then we had that fiasco of a PR. I think I'll stick to the command line from now on!

As for the tests I've been really busy too, and will be for another week or so, but I'll be on it again after that.

@edannenberg
Copy link
Owner

No worries, shit happens. :) I'm a bit out of order currently due to a few health issues, hopefully back on track soon.

I think I'll stick to the command line from now on!

Yep, with active bash completion it's probably more efficient then any gui anyways.

PPS: As a long term former Eclipse user I highly recommend all Jetbrains products, switched a few years ago and it's worth every penny. They even added shell scripting support with integrated shell check lately.

@HackingM
Copy link
Contributor Author

Sorry not to reply sooner but I've also been busy sadly. Sorry to hear about your health issues too - nothing too serious I hope.

I haven't managed to put together any tests for kubler yet but I have been playing around in one of my own repositories to see what I can do with the "new" GitHub Actions. If you want to see what I have managed so far have a look at https://github.com/MADhacking/hacking-bash-lib (shellcheck, unit tests & test coverage) and https://github.com/hacking-gentoo/gentoo/packages/41846

@edannenberg
Copy link
Owner

Hey Max, sorry for the radio silence. I had my (hopefully) last operation last week, I should have some time again after christmas to tackle this.

@HackingM
Copy link
Contributor Author

Hi Erik, no worries! Sounds serious - glad to hear you're hopefully on the mend.

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