Dispatch user prompts to dialog tool. :heart:

[gabyx]

Another try. Fixed the read -r VARIABLE

[coveralls]

Pull Request Test Coverage Report for Build 224

• 41 of 53 (77.36%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.4%) to 90.636%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
cli.sh	26	28	92.86%
base-template.sh	15	25	60.0%
<!--	Total:	41	53	77.36%	-->

Totals
Change from base Build 205:	-0.4%
Covered Lines:	1568
Relevant Lines:	1730

---

💛 - Coveralls

[gabyx]

I think there is a missing:

    sed -i 's|show_prompt TRUST_ALL_HOOKS.*|TRUST_ALL_HOOKS=\${TRUST_ALL_HOOKS} # disabled for tests: read -r TRUST_ALL_HOOKS|' /var/lib/githooks/base-template.sh

in exec-tests.sh... Test 34 is failing, strange? Do you know why...

[gabyx]

Is this test really correct?? https://github.com/rycus86/githooks/blob/df9e879fd9c834f6c23b7138e71025e69d8de235/tests/step-034.sh#L12

https://github.com/rycus86/githooks/blob/df9e879fd9c834f6c23b7138e71025e69d8de235/tests/step-034.sh#L22 You say here dont accept changes and it does not run the hooks..??

[gabyx]

I added a new test 104 and changed to your suggestions. Question: I found still some caveats, can we make the interface of the dialog tool a little better. Returnin the answer on stdout is a bit cumbersome, as we cannot switch inside the dialog if we have terminal for example and then print a prompt and so on...

        ANSWER=$(call_script "$DIALOG_TOOL" "githook::" "$TEXT" "$SHORT_OPTIONS" "$@")

• we could use the exit code > 0 for the success and answer
• we could use stderr for the answer (ugly)
• we could simply fallback to the read -r when the dialog returns exit code 1 (use default read -r), exit code > 1 means failure

Option 3: We can still detect in the dialog run if we are interactive or not...

Chose Option3 in afe1d38 because of simplicity

[gabyx]

@rycus86 : Could you fix the following issue: in coverage.sh you replace the inline content with the base-template (cat..)

# Replace the inline content with loading the source file
    python /var/lib/tests/replace-inline-content.py /var/lib/githooks && \\

And in this way running the install.sh it uses the mocked/sed-replaced content, and test-104 fails. Is that an error?; Maybe make a backup and use this in replace-inline-content.py I probably dont really get the test/coverage setup, maybe you could enlighten me a bit: As I see you want

• the install script not mocked, so everything is installed as we commited it (you only replace /dev/tty, to make the install run through... I think)
• the base-template.sh mocked for special tests and prompt overrides etc...

Fixed it in 823a5eb

[gabyx]

Hopefully ready to merge, if you are :-) Thx.

[gabyx]

You can squash merge :-) probably

[rycus86]

@rycus86 : Could you fix the following issue: in coverage.sh you replace the inline content with the base-template (cat..)

# Replace the inline content with loading the source file
    python /var/lib/tests/replace-inline-content.py /var/lib/githooks && \\

And in this way running the install.sh it uses the mocked/sed-replaced content, and test-104 fails. Is that an error?; Maybe make a backup and use this in replace-inline-content.py I probably dont really get the test/coverage setup, maybe you could enlighten me a bit: As I see you want

• the install script not mocked, so everything is installed as we commited it (you only replace /dev/tty, to make the install run through... I think)
• the base-template.sh mocked for special tests and prompt overrides etc...

Fixed it in 823a5eb

You can see the generated files we run the coverage against if you open the ./cover/index.html file in a browser then click through to the install script's coverage. The line with the cat will turn into this for example:

BASE_TEMPLATE_CONTENT=$(cat /var/lib/githooks/base-template.sh) #> BASE_TEMPLATE_CONTENT='#!/bin/bash

So this makes sure we use the contents of the base template and the cli script of the current commit.

I checked out your branch and have now worked out why it wasn't working. It's because we mock out the response of the script, without actually calling it. I'm OK with that for now, so please revert the last commit, and in the test, just have ACCEPT_CHANGES=Y plus make sure the dialog file exists at the expected location so the new code path is triggered.

[rycus86]

@gabyx left a few comments but otherwise looking good. :+1: Please do those changes and squash the commits when you can, then I'll merge this in after as soon as I have time. Thanks a lot!

[gabyx]

is the ./cover/index.html generated in the coverage.sh run? because I dont see any...

[rycus86]

is the ./cover/index.html generated in the coverage.sh run? because I dont see any...

Yeah, if you run sh tests/coverage.sh, it should generate it in the cover directory of the root of the project.

[gabyx]

Where would you set ACCEPT_CHANGES=Y? in Test step-104.sh I need to trigger one prompt to trigger the installed dialog tool. But the install.sh has every prompt mocked...

[rycus86]

Just on the commit instruction like ACCEPT_CHANGES=Y git commit ...

[rycus86]

So the actual dialog tool won't run, because that will be something the user chooses, so we don't need to test that. Maybe we could, I'm not sure.

Thinking about it now, perhaps we should only mock the read -r output and let the test dialog tool run if exists?

[rycus86]

No, running git commit should trigger the pre-commit hook just fine.

On Fri, 26 Jul 2019, 21:33 Gabriel Nützi, notifications@github.com wrote:

Ah I need to trigger the code by executing directly base-template.sh ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rycus86/githooks/pull/28?email_source=notifications&email_token=AAXWDWU7J7YBVXSDPSDYFPLQBLOHVA5CNFSM4IFYAHZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD24KN7Y#issuecomment-515417855, or mute the thread https://github.com/notifications/unsubscribe-auth/AAXWDWUJNYG7HGX4FOS6SMTQBLOHVANCNFSM4IFYAHZQ .

[gabyx]

Perfect, now it works.

[rycus86]

Looks good, thanks a lot! Will merge it in tomorrow. :tada:

[rycus86]

Sorry for the delay, just want to fix up some formatting after merge and can't find the time. Will try later today (I hope).

[gabyx]

Great :-)