Bug 26384 - Missing test to catch for execution flags
Summary: Missing test to catch for execution flags
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
: 26382 (view as bug list)
Depends on: 26401
Blocks:
  Show dependency treegraph
 
Reported: 2020-09-04 09:54 UTC by Jonathan Druart
Modified: 2020-10-15 21:29 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
Bug 26384: Add test to catch for missing or extra execution flags (2.18 KB, patch)
2020-09-07 09:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26384: Fix executable flags (86.85 KB, patch)
2020-09-07 09:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26384: Add test to catch for missing or extra execution flags (2.24 KB, patch)
2020-09-10 12:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26384: Fix executable flags (86.67 KB, patch)
2020-09-10 12:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26384: Add test to catch for missing or extra execution flags (2.24 KB, patch)
2020-09-10 12:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26384: Fix executable flags (88.47 KB, patch)
2020-09-11 07:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26384: Add test to catch for missing or extra execution flags (2.35 KB, patch)
2020-09-11 07:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26384: (QA follow-up) Fix two exceptions (1.25 KB, patch)
2020-09-11 07:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26384: Add .t extension to make the script executed by CI (710 bytes, patch)
2020-09-28 08:35 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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