Introduce tests, namespacing, precision rounding and optional exiting

[johanvanhelden]

Adds:

• Tests
• Precision rounding
• Optional exiting

And cleans the code up a lot.

Fixes #6 and #5

[johanvanhelden]

@emibcn can you confirm that the way rounding is implemented here is good for you?

And will this be a breaking change for you? If not, I will tag this as v1, if yes, I'll add a new v2 tag.

[emibcn]

@emibcn can you confirm that the way rounding is implemented here is good for you?

Wow!! Wasn't expecting such a change! Great!

About de precision: The method to get the rounded number looks goodI think it's better to have 2 outputs: one with all the decimals (for calculations) and another rounded (for pretty printing).

About the exit variable: it would make sense to me if you output another extra value (for the minimum coverage passed or not). I mean, if you set percentage: 70 but use exit: false, ¿what is the finality of executing the action? If you want to never get the action to failure, you set percentage: 0 and will still get the output printed in the workflow. Unless you want the action to give a bool output to some next step (for example, sending a Telegram message and blocking a merge).

About the Dockerfile: I think it will be easier to you to use the PHP file /src/test-coverage-checker.php directly as entrypoint of the Docker container. This way, you don't need to install bash and you don't need to add every parameter 3 times (one in the action inputs section, another in the action run and another one in the entrypoint). You only need to add the shebang to the file and add execution permissions (the commit will save that too).

I have been looking for a library to create GitHub Actions with PHP like the ones in JS (so you can get the inputs directly), but haven't found any, yet. Then, we still need to pass each input into the docker args in the action.yml. So, writing 2 times each input is the minimum.

And will this be a breaking change for you? If not, I will tag this as v1, if yes, I'll add a new v2 tag.

This is not a breaking change to me. The amount of lines and the approach have changed both a lot, maybe this is enough to re-tag? I don't know, it's up to you.

[johanvanhelden]

Thanks for the feedback @emibcn !

I'll try to release this tomorrow or in the weekend with your added suggestions as a v2 release.

The main reason for the exit option was to allow me to test things with PHPUnit and not having the script actually exit. But a failed boolean output is a great added feature.

I tried using a shebang and using the php file as an entrypoint, but ended up getting php errors because of the new namespacing. I'd rather have the new namespaced setup and having to declare duplicate inputs vs the old setup. But feel free to try and help me to find a solution 😄

[emibcn]

Thanks for the feedback @emibcn !

Thank you for the job!

I'll try to release this tomorrow or in the weekend with your added suggestions as a v2 release.

Great!

The main reason for the exit option was to allow me to test things with PHPUnit and not having the script actually exit. But a failed boolean output is a great added feature.

Fine!

I tried using a shebang and using the php file as an entrypoint, but ended up getting php errors because of the new namespacing. I'd rather have the new namespaced setup and having to declare duplicate inputs vs the old setup. But feel free to try and help me to find a solution

Did a pull request over your branch. After re-creating the Docker image, it works:

$ cat Dockerfile 
FROM php:7.4-alpine

COPY src /src

ENTRYPOINT ["/src/test-coverage-checker.php"]

$ docker build . --tag gha:latest
Sending build context to Docker daemon  402.9kB
Step 1/3 : FROM php:7.4-alpine
 ---> 13b8fc557e7d
Step 2/3 : COPY src /src
 ---> 96a3f8b5e70e
Step 3/3 : ENTRYPOINT ["/src/test-coverage-checker.php"]
 ---> Running in cee988e16801
Removing intermediate container cee988e16801
 ---> cd1b7d411c97
Successfully built cd1b7d411c97
Successfully tagged gha:latest

# Here, the volume is used to contain the XML file
$ docker run --volume $PWD/v:/v:ro gha:latest /v/clover.xml 40 2 true ; echo $?
[ERROR] Code coverage is 38.99%, which is not accepted (>=40%)
##[set-output name=coverage;]38.99
1

[emibcn]

Ahhh, ok, now I see the workflow output. Ok, let's try another solution.

https://github.com/johanvanhelden/gha-clover-test-coverage-check/pull/8/checks?check_run_id=1101926545

[emibcn]

Ok, now I've changed the entrypoint to pass all arguments, independently of how many are there, and use exec to handle exit status.

[johanvanhelden]

@emibcn

Thank you for the entrypoint polish, it's a lot better now!

And I am also finished now.

I made all changes fully backward-compatible so I'll just release it on v1 - this way I am not forcing anyone to update all their workflows to v2.

Please check the README for all the new input and output options. I tried to keep it extreme flexible, so people can just use what and how they want.

[emibcn]

This is a great job!! Going to update all the workflows where I use it ;)