Bug 34303 - t/00-testcritic.t should only test files part of git repo
Summary: t/00-testcritic.t should only test files part of git repo
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 34911
  Show dependency treegraph
 
Reported: 2023-07-18 10:45 UTC by Marcel de Rooy
Modified: 2023-12-28 20:47 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02,22.11.08,22.05.16


Attachments
Bug 34303: Only perlcritic files from git repo (783 bytes, patch)
2023-07-18 11:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34303: Only perlcritic files from git repo (801 bytes, patch)
2023-07-18 11:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34303: Loop over pl and pm from the git repo (884 bytes, patch)
2023-07-18 12:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34303: Only perlcritic files from git repo (829 bytes, patch)
2023-07-18 12:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34303: Only perlcritic files from git repo (819 bytes, patch)
2023-07-18 12:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34303: Only perlcritic files from git repo (976 bytes, patch)
2023-07-18 12:22 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 Marcel de Rooy 2023-07-18 10:45:40 UTC
See 33743 and 33745
Comment 1 Marcel de Rooy 2023-07-18 11:00:40 UTC
Created attachment 153588 [details] [review]
Bug 34303: Only perlcritic files from git repo

Test plan:
Run t/00-testcritic.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2023-07-18 11:58:48 UTC
Created attachment 153589 [details] [review]
Bug 34303: Only perlcritic files from git repo

Test plan:
Run t/00-testcritic.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2023-07-18 12:01:41 UTC
TODO [13:59] <Joubu> a dir in git index can contain files that are not in the git index
Comment 4 Jonathan Druart 2023-07-18 12:07:31 UTC
Created attachment 153590 [details] [review]
Bug 34303: Loop over pl and pm from the git repo

We don't want to include files that are not in the git index.
Comment 5 Jonathan Druart 2023-07-18 12:10:22 UTC
* This is way slower than using all_critic_ok (29sec vs 1m50).
To investigate...

* Should we include '.t' files?

* What about perl files without extensions? (were they tested before?)
Comment 6 Marcel de Rooy 2023-07-18 12:12:07 UTC
Created attachment 153591 [details] [review]
Bug 34303: Only perlcritic files from git repo

Test plan:
Run t/00-testcritic.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2023-07-18 12:14:02 UTC
(In reply to Jonathan Druart from comment #5)
> * This is way slower than using all_critic_ok (29sec vs 1m50).
> To investigate...
We can use all. See follow-up

> * Should we include '.t' files?
They are in now.

> * What about perl files without extensions? (were they tested before?)
Checking
Comment 8 Marcel de Rooy 2023-07-18 12:15:35 UTC
all_perl in the library checks first extension but also shebang line

return 1 if defined $first && ( $first =~ m{ \A [#]!.*perl }xms );
Comment 9 Marcel de Rooy 2023-07-18 12:17:02 UTC
Created attachment 153592 [details] [review]
Bug 34303: Only perlcritic files from git repo

Test plan:
Run t/00-testcritic.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Jonathan Druart 2023-07-18 12:22:03 UTC
Created attachment 153596 [details] [review]
Bug 34303: Only perlcritic files from git repo

Test plan:
Run t/00-testcritic.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

JD amended patch: tidy

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Jonathan Druart 2023-07-18 12:22:11 UTC
Noting this warning (not introduced by this patch):
Use of uninitialized value $_ in pattern match (m//) at /usr/share/perl5/PPIx/Regexp/Structure/Quantifier.pm line 117.
Comment 12 Tomás Cohen Arazi 2023-07-18 17:09:09 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 13 Martin Renvoize 2023-07-19 08:30:49 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 14 Matt Blenkinsop 2023-07-19 10:15:42 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x
Comment 15 Lucas Gass 2023-09-13 20:45:33 UTC
Backported to 22.05.x for upcoming 22.05.16