Bug 32352 - xt/check_makefile.t failing on node_modules
Summary: xt/check_makefile.t failing on node_modules
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-11-25 11:24 UTC by Jonathan Druart
Modified: 2022-11-29 14:09 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00, 22.11.01


Attachments
Bug 32352: Make xt/check_makefile.t check directories in git index (1.23 KB, patch)
2022-11-25 11:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32352: Make xt/check_makefile.t check directories in git index (1.28 KB, patch)
2022-11-28 14:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2022-11-25 11:24:20 UTC
21:55:20 koha_1       | #   Failed test 'All directories must be listed in Makefile.PL'
21:55:20 koha_1       | #   at xt/check_makefile.t line 39.
21:55:20 koha_1       | #          got: '1'
21:55:20 koha_1       | #     expected: '0'
21:55:20 koha_1       | # $VAR1 = [
21:55:20 koha_1       | #           './node_modules'
21:55:20 koha_1       | #         ];
Comment 1 Jonathan Druart 2022-11-25 11:27:39 UTC
Created attachment 144242 [details] [review]
Bug 32352: Make xt/check_makefile.t check directories in git index

It's failing if 'node_modules' exists. We could simply skip it, but it
seems that it would be better to actually list directories that are part
of the git index only.

The question is: do we actually support test execution outside of a git
directory?
Comment 2 Marcel de Rooy 2022-11-26 07:24:00 UTC
Just a dumb question :) What about:
The dir should exist, right?

diff --git a/Makefile.PL b/Makefile.PL
index 131d8a0ae2..9d4eb9d604 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -346,6 +346,7 @@ my $target_map = {
   './misc/translator'           => { target => 'MISC_DIR', trimdir => 2 },
   './misc/koha-install-log'     => { target => 'MISC_DIR', trimdir => -1 },
   './misc/installer_devel_notes' => 'NONE',
+  './node_modules'              => 'INTRANET_CGI_DIR',
   './opac'                      => 'OPAC_CGI_DIR',
   './OpenILS'                   => 'PERL_MODULE_DIR',
   './README.txt'                => 'NONE',
Comment 3 Jonathan Druart 2022-11-26 08:54:12 UTC
No, it shouldn't in production env.
Comment 4 Jonathan Druart 2022-11-26 08:55:14 UTC
And it's not part of git index, so not necessarily exists on dev env either.
Comment 5 Martin Renvoize 2022-11-28 09:55:59 UTC
Checking the git tree feels like the right approach.

Must admit, I've never tried to run the tests from outside of the kohaclone.. but that's doesn't mean there isn't someone out there doing that.

I think I'd suggest going with this patch and documenting that the expectation is that tests are run from inside the project somewhere.  I can't see any obvious flaws with this.
Comment 6 Tomás Cohen Arazi 2022-11-28 13:17:51 UTC
I agree only checking things in the git tree is the right approach.
Comment 7 Tomás Cohen Arazi 2022-11-28 14:28:36 UTC
Created attachment 144283 [details] [review]
Bug 32352: Make xt/check_makefile.t check directories in git index

It's failing if 'node_modules' exists. We could simply skip it, but it
seems that it would be better to actually list directories that are part
of the git index only.

The question is: do we actually support test execution outside of a git
directory?

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Tomás Cohen Arazi 2022-11-28 18:37:56 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 9 Martin Renvoize 2022-11-29 14:09:29 UTC
Pushed to 22.11.x for 22.11.01 release.

Thanks everyone.