search

eficode-academy / git-katas

A set of exercises for deliberate Git Practice
MIT License
1.32k stars 860 forks source link

Add feature to test all kata setup.sh scripts #299

Closed JKrag closed 3 years ago

JKrag commented 3 years ago

This PR includes addition of a test-al.sh script that test-runs the setup.sh of ALL katas, not just the basic-commit one.

To test all katas, run 

./test-all.sh

from the root folder.

It also includes a ./utils/test/test_setup.sh can be used to test a single kata (e.g. during development) Run it from the kata folder as:

../utils/test/test_setup.sh

While doing this, I noticed that the existing test.sh script currently used by the pipeline doesn't actually fail at all if there are problems. So this has now been modified to use the new test_setup.sh script and now fails as it should. (but test.sh still only tests the single basic-commits setup).

NOTE: The obvious omission in this PR is that the pipeline has not been changed to run the test-all.sh script as it obviously should. The reason for this is that I still need to figure out how to get test-all.sh to "collect" a summary and exit non-zero if there was one or more tests failing. But I figured the rest was valuable enough to get added now as is.

Initial work on this PR was done by me, but I want to give a shout-out to @zanderhavgaard for helping me debug and fix the error handling to get it working correctly.

JKrag commented 3 years ago

@atombrella Thanks for the function rewrite. This was exactly what I was hoping for, and works a charm (tested on both zsh and bash on Mac so far. Apart from a tiny commit with some ShellCheck linting improvements, I also felt inspired and quickly added some counters and reporting of total number of failed/completed tests. Maybe a bit crude, but it felt like it was needed for "completeness" :-)

JKrag commented 3 years ago

I have now added the running of test-all.sh to the pipeline, as was the original intention. Unfortunately this fails on Ubuntu because of the default branch warning. I have already addressed this issue in another branch, so I guess I will have to add that change to here to make it work. It does muddy the water a bit, but the change was destined to be added immediately after this one anyway.

JKrag commented 3 years ago

I added my default branch Handling code, and now it just fails instead on missing user.name & user.email, but still only on Ubuntu. Lets try to fix that as well.

JKrag commented 3 years ago

Finally 🟢 with all tests. I will have to address the issue on running test-all.sh in ZSH later. Simply changing the Dockerfile to ./test-all.sh is not enough, as that would still execute the content with bash as per the shebang line.