Bug 26384

Summary: Missing test to catch for execution flags
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: dcook, fridolin.somers, lucas, m.de.rooy, martin.renvoize
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26382
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26331
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00
Bug Depends on: 26401    
Bug Blocks:    
Attachments: Bug 26384: Add test to catch for missing or extra execution flags
Bug 26384: Fix executable flags
Bug 26384: Add test to catch for missing or extra execution flags
Bug 26384: Fix executable flags
Bug 26384: Add test to catch for missing or extra execution flags
Bug 26384: Fix executable flags
Bug 26384: Add test to catch for missing or extra execution flags
Bug 26384: (QA follow-up) Fix two exceptions
Bug 26384: Add .t extension to make the script executed by CI

Description Jonathan Druart 2020-09-04 09:54:45 UTC
We need to catch missing execution flags for some scripts, like the ones in svc and misc

It could be good to have a qa-test-tools check as well.
Comment 1 David Cook 2020-09-07 00:40:28 UTC
I was thinking this as well. There are some maintenance scripts that probably don't *need* the execute permission, but it wouldn't hurt to add it to them.
Comment 2 Jonathan Druart 2020-09-07 09:32:33 UTC
*** Bug 26382 has been marked as a duplicate of this bug. ***
Comment 3 Jonathan Druart 2020-09-07 09:34:48 UTC
Created attachment 109691 [details] [review]
Bug 26384: Add test to catch for missing or extra execution flags
Comment 4 Jonathan Druart 2020-09-07 09:34:53 UTC
Created attachment 109692 [details] [review]
Bug 26384: Fix executable flags

.pm must not have -x
.t must have -x
.pl must have -x

Test plan:
Apply only the first patch, run the tests and confirm that the failures
make sense
Apply this patch and confirm that the test now returns green
Comment 5 Martin Renvoize 2020-09-10 12:02:52 UTC
Created attachment 109845 [details] [review]
Bug 26384: Add test to catch for missing or extra execution flags

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2020-09-10 12:03:27 UTC
Good call, test works perfectly.. Final patch no longer applies as one of the touched files is no longer present.
Comment 7 Jonathan Druart 2020-09-10 12:08:40 UTC
Created attachment 109847 [details] [review]
Bug 26384: Fix executable flags

.pm must not have -x
.t must have -x
.pl must have -x

Test plan:
Apply only the first patch, run the tests and confirm that the failures
make sense
Apply this patch and confirm that the test now returns green
Comment 8 Jonathan Druart 2020-09-10 12:08:44 UTC
Created attachment 109848 [details] [review]
Bug 26384: Add test to catch for missing or extra execution flags

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Marcel de Rooy 2020-09-11 07:14:28 UTC
Created attachment 109884 [details] [review]
Bug 26384: Fix executable flags

.pm must not have -x
.t must have -x
.pl must have -x

Test plan:
Apply only the first patch, run the tests and confirm that the failures
make sense
Apply this patch and confirm that the test now returns green

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2020-09-11 07:14:33 UTC
Created attachment 109885 [details] [review]
Bug 26384: Add test to catch for missing or extra execution flags

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2020-09-11 07:14:39 UTC
Created attachment 109886 [details] [review]
Bug 26384: (QA follow-up) Fix two exceptions

AuthUtils.t went back to 644, but should be 755
Adding an exception in the xt test for fix-old-fsf-address.exclude

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Jonathan Druart 2020-09-11 07:59:06 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 13 Jonathan Druart 2020-09-28 08:35:59 UTC
Created attachment 110834 [details] [review]
Bug 26384: Add .t extension to make the script executed by CI

The test file is not run as it does not have the .t extension
Comment 14 Jonathan Druart 2020-10-12 11:56:52 UTC
I have added a test in the QA script.

https://gitlab.com/koha-community/qa-test-tools/-/issues/39
Comment 15 Lucas Gass 2020-10-15 21:29:24 UTC
does not apply cleanly to 20.05.x, no backport