Calling bash-unit pre-commit hook in ci + sample setup

[mdeweerd]

This is an update to the pre-commit configuration so that it uses the hook proposed in this repository.

I used a specific commit has as there is/was no regular tag yet.

This alos shows a limitation of pre-commit: only changed files are provides as arguments and there is no option to automatically propose all files matching the regex.

Howevert that means that one could execute bash_unit on a test that changed automatically.
As shown in the integration setup, it's still possible to run on all files usin the '-a/--all-files' option. The other solution is to use the 'args:' parameter in the hook configuration.

[pgrange]

Hi @mdeweerd,

Thank you for your PR. First, I find the quality gates that pre-commit is checking on bash_unit already, thanks to your setup, super handy and very efficient, thank you again.

As for running the tests per se, I'm not yet comfortable with doing that. I'm not sure I want the tests to be just another quality gate. I mean, I can live with end of file line issues, with quotes confusing usage, etc. but there's no way we can have failing tests, never. So I appreciate the idea of having both pre-commit on one side and the tests on the other side as first class citizens and not the tests lost in the pre-commit quality gates.

In summary: I love the idea of having two distinct C.I. steps: one for the tests, another for pre-commit.

This PR also helps me realize that we might have an issue with bash_unit as a pre-commit anyway. As a developer writing code and testing it with bash_unit what I want is for the tests to run when the production code is changed. Running the tests when I change the test is important of course but when the production code is changed, this is when I want to have the tests ran. And as you write in this PR's description, pre-commit will only run on changed files.

In this particular instance, what I want is for all the tests to run when the file bash_unit is changed. So for pre-commit + bash_unit to be very useful I would probably want to say something like:

When these files change, run these tests

But maybe we're a bit out of scope of what a pre-commit hook should do? As you mention, we can run pre-commit run --all-files but doesn't It defeat pre-commit purpose if we do that for every commit?

I'm just thinking out loud here and wondering how one would use bash_unit with pre-commit. Not being a user myself this does not have strong value so I wouldn't change your contribution about it in #100 you certainly nows it better than I do.

Anyway, not a fan of letting pre-commit run all the tests. I have also other bad reasons to not do it as bash_unit is having issues on BSD systems currently. So when coding on BSD (OS X in my case) you might need to have an alternative setup to run the tests like docker, for instance. So pre-commit would be a problem here... until BSD execution issues are solved, of course.

On a side note, I've tried to use it anyway but I can't make it work seemingly. If I run pre-commit on all files I don't see any bash_unit step:

#> git switch use_precommit_hook 
Switched to branch 'use_precommit_hook'
Your branch is up to date with 'mdeweerd/use_precommit_hook'.
#> pre-commit run --all-files
don't commit to branch...................................................Passed
check yaml...............................................................Passed
check json...........................................(no files to check)Skipped
mixed line ending........................................................Passed
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check for merge conflicts................................................Passed
check that executables have shebangs.....................................Passed
check that scripts with shebangs are executable..........................Passed
fix utf-8 byte order marker..............................................Passed
check for case conflicts.................................................Passed
beautysh.................................................................Passed
Run local script before commit if it exists..............................Passed
yamllint.................................................................Passed
codespell................................................................Passed
shellcheck...............................................................Passed

[mdeweerd]

Overall I agree and what I did here is probably inline with your observations.

The reasons to add bash-commit to the pre-commit configuration are:

• Provide a demo implementation for this project's own hook and run it in ci which is a means to validate it works;
• Provide a simple means to run the hook from the CLI, when we want to (the hook also ensures the configuration of bash_unit making it a "no-brainer" to run bash_unit - no need to lookup the download link, make the script executable, etc). This is less valid for this project as the script is in the project itself, but it is for other projects;
• Provide a simple means to run the tests in ci. As you can verify it is just an extra line. No need to add more lines to download bash_unit, etc. It's all done by pre-commit.

The hook does not run by default as I defined that it should run only in the "manual" stage. When you run pre-commit from the CLI without the --hook-stage option it will not run.
Tests are generally quite long and you do not want to wait for them to make the commit - I think you mention that in your observations. Except for quick tests, I add them to pre-commit as manual.

Documentation and 'ci' do the rest to run the tests. Again, having it as a pre-commit configuration makes it easy to setup and update - one just needs to change the revision number and the project is using another bash_unit version.

Using pre-commit for the configuration also allows that version control of the tool used. While I admit that one could integrate the bash_unit version in the link to download it from, the readme does propose to install it from "master", and the install.sh script will just get the latest release.
pre-commit allows the version control and ensure that tests run even after breaking changes to the latest bash_unit version that was released.

So basically I think we agree, it is just a matter of using pre-commit in a way that matches that.

EDIT:

• Pre-commit will not test the latest bash_unit version as it refers to a version number, so the separated ci workflow is still valid here. However, for other repositories that use the pre-commit hook for bash_unit, that is not true. They want to use a stable bash_unit version, not test bash_unit.

EDIT2:

• Possibly pre-commit try-repo [...] could be used in ci to test that the current version of the hook and the repo and bash_unit are fine.

[mdeweerd]

I've added a pre-commit try-repo line to the pre-commit.yaml workflow.

[pgrange]

Thanks for your detailed answer @mdeweerd , it all makes sense to me 👍