[Bug 39115] New: Tidy script should list the files we do not want to tidy
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Bug ID: 39115 Summary: Tidy script should list the files we do not want to tidy Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@gmail.com Reporter: jonathan.druart@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 38664 It should prevent to tidy files that must be kept unchanged. It was initially a feature (the script does what you ask it to do), but it's a mess maintain the list of files we want to keep tidy. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38664 [Bug 38664] Tidy the whole codebase -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #1 from Victor Grousset/tuxayo <victor@tuxayo.net> --- The 4 places that have/need the denylist misc/devel/tidy.pl push @files, qx{git ls-files '*.pl' '*.pm' '*.t' ':(exclude)Koha/Schema/Result' ':(exclude)Koha/Schema.pm'}; xt/perltidy.t push @files, qx{git ls-files '*.pl' '*.pm' '*.t' ':(exclude)Koha/Schema/Result' ':(exclude)Koha/Schema.pm'}; KTD: pre-commit hook: https://gitlab.com/koha-community/koha-testing-docker/-/issues/478 QA tools: tidy check: https://gitlab.com/koha-community/qa-test-tools/-/issues/91 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #2 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 177971 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177971&action=edit Bug 39115: Do not tidy files that must not be touched It should prevent to tidy files that must be kept unchanged. It was initially a feature (the script does what you ask it to do), but it's a mess maintain the list of files we want to keep tidy. Test plan: Koha/Schema.pm is one of the file we don't want to modify/tidy So let's play with this one, feel free to pick others (see the exception list per filetype in the script) % perl misc/devel/tidy.pl --perl will not touch Koha/Schema.pm % perl misc/devel/tidy.pl mainpage.pl Koha/Schema.pm will not touch Koha/Schema.pm but mainpage.pl will % perl misc/devel/tidy.pl --no-write Koha/Schema.pm The --no-write option is called by the QA script to compare the output and see if files are tidy. We want to return something in this case. So the file will be printed to STDOUT untouched -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #3 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 177972 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177972&action=edit Bug 39115: Remove exclude list from tt_tidy.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #4 from Victor Grousset/tuxayo <victor@tuxayo.net> --- WIP testing results: I added some untidy stuff in Koha/Schema.pm and with the patches it's preserved :D Output of the script: 2955 files on main 2952 files with the patches Diffing the output gives this files that are not here anymore Koha/Schema.pm: good opac/svc/shelfbrowser.pl not being listed twice svc/cataloguing/automatic_linker.pl not being listed twice Weird but ok. I'll update with the rest of the testing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177971|0 |1 is obsolete| | --- Comment #5 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 177979 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177979&action=edit Bug 39115: Do not tidy files that must not be touched It should prevent to tidy files that must be kept unchanged. It was initially a feature (the script does what you ask it to do), but it's a mess maintain the list of files we want to keep tidy. Test plan: Koha/Schema.pm is one of the file we don't want to modify/tidy So let's play with this one, feel free to pick others (see the exception list per filetype in the script) % perl misc/devel/tidy.pl --perl will not touch Koha/Schema.pm % perl misc/devel/tidy.pl mainpage.pl Koha/Schema.pm will not touch Koha/Schema.pm but mainpage.pl will % perl misc/devel/tidy.pl --no-write Koha/Schema.pm The --no-write option is called by the QA script to compare the output and see if files are tidy. We want to return something in this case. So the file will be printed to STDOUT untouched Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177972|0 |1 is obsolete| | --- Comment #6 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 177980 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177980&action=edit Bug 39115: Remove exclude list from tt_tidy.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #7 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Rest of the test plan: ok Confirmed it fixes directly: #478 QA tools: pre-commit hook shouldn't check tidyness on DBIX schema files https://gitlab.com/koha-community/koha-testing-docker/-/issues/478 Confirmed it fixes directly: #91 KTD: tidyness check shouldn't happen on DBIX schema files https://gitlab.com/koha-community/qa-test-tools/-/issues/91 testing notes: have a KTD repo up to date (for hooks) And when having started KTD: cd ../qa-test-tools/ ; sudo git pull # to update the QA tools -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #8 from Victor Grousset/tuxayo <victor@tuxayo.net> --- There is this separate occurrence of the perl denylist that is now diverging from tidy.pl: xt/perltidy.t 7:push @files, qx{git ls-files '*.pl' '*.pm' '*.t' ':(exclude)Koha/Schema/Result'}; (missing Koha/Schema.pm) js_tidy.t also has duplication of the list: qx{git ls-files '*.js' '*.ts' ':(exclude)koha-tmpl/intranet-tmpl/lib' ':(exclude)koha-tmpl/intranet-tmpl/js/Gettext.js' ':(exclude)koha-tmpl/opac-tmpl/lib' ':(exclude)Koha/ILL/Backend/'}; tt_tidy.t is good because it calls tidy.pl. That's why you could remove the denylist from there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #9 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 177984 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177984&action=edit Bug 39115: Remove exclude list from js_tidy.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |39119 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39119 [Bug 39119] xt/js_tidy.t and xt/vue_tidy.t does not use tidy.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #10 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 177985 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177985&action=edit Bug 39115: Replace (:exclude) with --exclude I don't really explain this one, but this is something wrong with (:exclude) % git ls-files koha-tmpl/intranet-tmpl/js/i18n.js --exclude='koha-tmpl/intranet-tmpl/lib' --exclude='koha-tmpl/intranet-tmpl/js/Gettext.js' --exclude='koha-tmpl/opac-tmpl/lib' koha-tmpl/intranet-tmpl/js/i18n.js => Return the file has expected % git ls-files koha-tmpl/intranet-tmpl/js/i18n.js ':(exclude)koha-tmpl/intranet-tmpl/lib' ':(exclude)koha-tmpl/intranet-tmpl/js/Gettext.js' ':(exclude)koha-tmpl/opac-tmpl/lib' => Does not return anything! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #11 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Jonathan Druart from comment #10)
Created attachment 177985 [details] [review] Bug 39115: Replace (:exclude) with --exclude
I don't really explain this one, but this is something wrong with (:exclude)
% git ls-files koha-tmpl/intranet-tmpl/js/i18n.js --exclude='koha-tmpl/intranet-tmpl/lib' --exclude='koha-tmpl/intranet-tmpl/js/Gettext.js' --exclude='koha-tmpl/opac-tmpl/lib' koha-tmpl/intranet-tmpl/js/i18n.js => Return the file has expected
% git ls-files koha-tmpl/intranet-tmpl/js/i18n.js ':(exclude)koha-tmpl/intranet-tmpl/lib' ':(exclude)koha-tmpl/intranet-tmpl/js/Gettext.js' ':(exclude)koha-tmpl/opac-tmpl/lib' => Does not return anything!
It seems that * --exclude filters after resolving the explicit file path * :(exclude) filters before resolving the file path, so even explicitly listed files can be excluded -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #12 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 178003 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178003&action=edit Bug 39115: Restore ability to tidy files not in git index (yet) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #13 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Jonathan Druart from comment #12)
Created attachment 178003 [details] [review] Bug 39115: Restore ability to tidy files not in git index (yet)
For instance: perl misc/devel/tidy.pl --no-write koha-tmpl/intranet-tmpl/prog/fr-FR/modules/intranet-main.tt perl misc/devel/tidy.pl koha-tmpl/intranet-tmpl/prog/fr-FR/modules/intranet-main.tt perl misc/devel/tidy.pl koha-tmpl/intranet-tmpl/prog/fr-FR/modules/intranet-main.tt koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt Compare the result with and without the patch. Confirm that the new result are the expected one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |major -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |39130 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39130 [Bug 39130] Add unit tests for xgettext.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #14 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Jonathan Druart from comment #13)
(In reply to Jonathan Druart from comment #12)
Created attachment 178003 [details] [review] [review] Bug 39115: Restore ability to tidy files not in git index (yet)
For instance:
perl misc/devel/tidy.pl --no-write koha-tmpl/intranet-tmpl/prog/fr-FR/modules/intranet-main.tt
perl misc/devel/tidy.pl koha-tmpl/intranet-tmpl/prog/fr-FR/modules/intranet-main.tt
perl misc/devel/tidy.pl koha-tmpl/intranet-tmpl/prog/fr-FR/modules/intranet-main.tt koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt
Compare the result with and without the patch. Confirm that the new result are the expected one.
But this is not working % git ls-files Koha/Schema.pm --exclude='Koha/Schema/Result' --exclude='Koha/Schema.pm' Koha/Schema.pm -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #15 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 178017 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178017&action=edit Bug 39115: Finally use ':(exclude)' and grep This was not working: % git ls-files Koha/Schema.pm --exclude='Koha/Schema/Result' --exclude='Koha/Schema.pm' Koha/Schema.pm The efficiency of the script is getting worse and it will need to be rewritten. The xt/tt_tidy.t will take longer. But at least it should work correctly now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff --- Comment #16 from Jonathan Druart <jonathan.druart@gmail.com> --- You will need this patch to the pre-commit hook if you want to test the commit. https://gitlab.com/koha-community/koha-testing-docker/-/commit/9b82c2d5417c4... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |39136 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39136 [Bug 39136] Tidy script should improve how exception list is built -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |Main Keywords| |RM_priority -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #17 from Victor Grousset/tuxayo <victor@tuxayo.net> ---
The efficiency of the script is getting worse and it will need to be rewritten. The xt/tt_tidy.t will take longer.
With and without the patches I get around 1110 seconds of time needed to run this locally. Unless there is something else to do to see the issue, there is no worry about these patches making the CI time even longer. ---- But yesterday, it was taking 600 secs to run this. So another fresh patch might have slowed the test. I'll also dig into this after this ticket. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177979|0 |1 is obsolete| | Attachment #177980|0 |1 is obsolete| | Attachment #177984|0 |1 is obsolete| | Attachment #177985|0 |1 is obsolete| | Attachment #178003|0 |1 is obsolete| | Attachment #178017|0 |1 is obsolete| | --- Comment #18 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 178048 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178048&action=edit Bug 39115: Do not tidy files that must not be touched It should prevent to tidy files that must be kept unchanged. It was initially a feature (the script does what you ask it to do), but it's a mess maintain the list of files we want to keep tidy. Test plan: Koha/Schema.pm is one of the file we don't want to modify/tidy So let's play with this one, feel free to pick others (see the exception list per filetype in the script) % perl misc/devel/tidy.pl --perl will not touch Koha/Schema.pm % perl misc/devel/tidy.pl mainpage.pl Koha/Schema.pm will not touch Koha/Schema.pm but mainpage.pl will % perl misc/devel/tidy.pl --no-write Koha/Schema.pm The --no-write option is called by the QA script to compare the output and see if files are tidy. We want to return something in this case. So the file will be printed to STDOUT untouched Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #19 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 178049 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178049&action=edit Bug 39115: Remove exclude list from tt_tidy.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #20 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 178050 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178050&action=edit Bug 39115: Remove exclude list from js_tidy.t Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #21 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 178051 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178051&action=edit Bug 39115: Replace (:exclude) with --exclude I don't really explain this one, but this is something wrong with (:exclude) % git ls-files koha-tmpl/intranet-tmpl/js/i18n.js --exclude='koha-tmpl/intranet-tmpl/lib' --exclude='koha-tmpl/intranet-tmpl/js/Gettext.js' --exclude='koha-tmpl/opac-tmpl/lib' koha-tmpl/intranet-tmpl/js/i18n.js => Return the file has expected % git ls-files koha-tmpl/intranet-tmpl/js/i18n.js ':(exclude)koha-tmpl/intranet-tmpl/lib' ':(exclude)koha-tmpl/intranet-tmpl/js/Gettext.js' ':(exclude)koha-tmpl/opac-tmpl/lib' => Does not return anything! Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #22 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 178052 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178052&action=edit Bug 39115: Restore ability to tidy files not in git index (yet) Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #23 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 178053 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178053&action=edit Bug 39115: Finally use ':(exclude)' and grep This was not working: % git ls-files Koha/Schema.pm --exclude='Koha/Schema/Result' --exclude='Koha/Schema.pm' Koha/Schema.pm The efficiency of the script is getting worse and it will need to be rewritten. The xt/tt_tidy.t will take longer. But at least it should work correctly now Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com --- Comment #24 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- # Failed test 'koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/tables/shelves_results.tt should be kept tidy' # at /kohadevbox/koha/xt/tt_tidy.t line 46. # Failed test 't/db_dependent/misc/translator/sample.tt should be kept tidy' # at /kohadevbox/koha/xt/tt_tidy.t line 46. # Looks like you failed 2 tests of 828. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/828 subtests Test Summary Report ------------------- /kohadevbox/koha/xt/tt_tidy.t (Wstat: 512 (exited 2) Tests: 828 Failed: 2) Failed tests: 679, 822 Non-zero exit status: 2 Files=1, Tests=828, 137 wallclock secs ( 0.05 usr 0.01 sys + 1594.13 cusr 306.97 csys = 1901.16 CPU) Result: FAIL I got the same failure with and without these patches, all else worked great, applied and tested hook -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #25 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Nick Clemens (kidclamp) from comment #24)
# Failed test 'koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/tables/ shelves_results.tt should be kept tidy' # at /kohadevbox/koha/xt/tt_tidy.t line 46.
# Failed test 't/db_dependent/misc/translator/sample.tt should be kept tidy' # at /kohadevbox/koha/xt/tt_tidy.t line 46. # Looks like you failed 2 tests of 828. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/828 subtests
Test Summary Report ------------------- /kohadevbox/koha/xt/tt_tidy.t (Wstat: 512 (exited 2) Tests: 828 Failed: 2) Failed tests: 679, 822 Non-zero exit status: 2 Files=1, Tests=828, 137 wallclock secs ( 0.05 usr 0.01 sys + 1594.13 cusr 306.97 csys = 1901.16 CPU) Result: FAIL
I got the same failure with and without these patches, all else worked great, applied and tested hook
My guess it that you don't have the new version of the prettier plugin, try `yarn install` or remove your local node_modules and pull the new ktd images. https://gitlab.com/koha-community/prettier-plugin-template-toolkit/-/commits... You need 2.0.2 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178048|0 |1 is obsolete| | --- Comment #26 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 178108 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178108&action=edit Bug 39115: Do not tidy files that must not be touched It should prevent to tidy files that must be kept unchanged. It was initially a feature (the script does what you ask it to do), but it's a mess maintain the list of files we want to keep tidy. Test plan: Koha/Schema.pm is one of the file we don't want to modify/tidy So let's play with this one, feel free to pick others (see the exception list per filetype in the script) % perl misc/devel/tidy.pl --perl will not touch Koha/Schema.pm % perl misc/devel/tidy.pl mainpage.pl Koha/Schema.pm will not touch Koha/Schema.pm but mainpage.pl will % perl misc/devel/tidy.pl --no-write Koha/Schema.pm The --no-write option is called by the QA script to compare the output and see if files are tidy. We want to return something in this case. So the file will be printed to STDOUT untouched Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178049|0 |1 is obsolete| | --- Comment #27 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 178109 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178109&action=edit Bug 39115: Remove exclude list from tt_tidy.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178050|0 |1 is obsolete| | --- Comment #28 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 178110 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178110&action=edit Bug 39115: Remove exclude list from js_tidy.t Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178051|0 |1 is obsolete| | --- Comment #29 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 178111 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178111&action=edit Bug 39115: Replace (:exclude) with --exclude I don't really explain this one, but this is something wrong with (:exclude) % git ls-files koha-tmpl/intranet-tmpl/js/i18n.js --exclude='koha-tmpl/intranet-tmpl/lib' --exclude='koha-tmpl/intranet-tmpl/js/Gettext.js' --exclude='koha-tmpl/opac-tmpl/lib' koha-tmpl/intranet-tmpl/js/i18n.js => Return the file has expected % git ls-files koha-tmpl/intranet-tmpl/js/i18n.js ':(exclude)koha-tmpl/intranet-tmpl/lib' ':(exclude)koha-tmpl/intranet-tmpl/js/Gettext.js' ':(exclude)koha-tmpl/opac-tmpl/lib' => Does not return anything! Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178052|0 |1 is obsolete| | --- Comment #30 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 178112 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178112&action=edit Bug 39115: Restore ability to tidy files not in git index (yet) Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178053|0 |1 is obsolete| | --- Comment #31 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 178113 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178113&action=edit Bug 39115: Finally use ':(exclude)' and grep This was not working: % git ls-files Koha/Schema.pm --exclude='Koha/Schema/Result' --exclude='Koha/Schema.pm' Koha/Schema.pm The efficiency of the script is getting worse and it will need to be rewritten. The xt/tt_tidy.t will take longer. But at least it should work correctly now Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | Status|Signed Off |Passed QA --- Comment #32 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Makes sense. No QA script failures. Tests pass with the latest KTD as noted by Joubu. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #33 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Additional testing with these files to confirm the pre-commit hook and the QA script ignores them as expected: Koha/Schema.pm Koha/Schema/Result/City.pm koha-tmpl/intranet-tmpl/lib/greybox/AJS.js koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-open.inc For other files, the hook tidies them as expected. The script QA doesn't complain but that will be a ticket I'll open for the QA tools. The perl part of the denylist is still duplicated with another instance still in xt/perltidy.t That will be for a follow-up ticket I'll open. To either remove that last redundancy. Or have a comment mentioning it to keep both in sync. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #34 from Victor Grousset/tuxayo <victor@tuxayo.net> ---
The script QA doesn't complain but that will be a ticket I'll open for the QA tools.
Oh no, actually the QA script not complaining about legit tidy issues is partially from these patches! --on main:------------------------------ FAIL koha-tmpl/intranet-tmpl/js/i18n.js FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl /kohadevbox/koha/koha-tmpl/intranet-tmpl/js/i18n.js` OK koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt FAIL mainpage.pl FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl /kohadevbox/koha/mainpage.pl` ----------------------- It was already missing intranet-main.tt. But it was working for the .pl and .js files. So that's a regression these patches will be introducing :/ ------------------------ Testing details in case I missed something: ----QA tools is on latest commit in my KTD--------------------------------- commit 11502a72aa1c77f9c981299a81a6d434e54e2808 Issue #90: Fix prettierrc.js filename -----name and diff of the files I untidied------------------------ diff --git a/koha-tmpl/intranet-tmpl/js/i18n.js b/koha-tmpl/intranet-tmpl/js/i18n.js index 6eb7e86a8d..d6a7ef3f5b 100644 --- a/koha-tmpl/intranet-tmpl/js/i18n.js +++ b/koha-tmpl/intranet-tmpl/js/i18n.js @@ -46,6 +46,6 @@ function __np(msgctxt, msgid, msgid_plural, count) { return Koha.i18n.gt.npgettext(msgctxt, msgid, msgid_plural, count); } -function __npx(msgctxt, msgid, msgid_plural, count, vars) { + function __npx(msgctxt, msgid, msgid_plural, count, vars) { return Koha.i18n.expand(__np(msgctxt, msgid, msgid_plural, count), vars); } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt index 91172dac2a..69faa7ff92 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt @@ -340,6 +340,6 @@ }); }); </script> -[% END %] + [% END %] <!-- the main div is closed in intranet-bottom.inc --> [% INCLUDE 'intranet-bottom.inc' %] diff --git a/mainpage.pl b/mainpage.pl index 87f784631f..1d948089aa 100755 --- a/mainpage.pl +++ b/mainpage.pl @@ -153,7 +153,7 @@ if ( C4::Context->preference('PatronSelfRegistrationAlert') ) { ); } -$template->param( + $template->param( pendingcomments => $pendingcomments, pendingtags => $pendingtags, pending_borrower_modifications => $pending_borrower_modifications, -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #35 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Victor Grousset/tuxayo from comment #34)
The script QA doesn't complain but that will be a ticket I'll open for the QA tools.
I actually fixed that yesterday but forgot to push it, and it was only inside ktd as I didn't find the commit on my host. It's caused by the grep -v and the use of abspath by the QA script. See https://gitlab.com/koha-community/qa-test-tools/-/commit/2c62b862ef264d9fb42... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #36 from Victor Grousset/tuxayo <victor@tuxayo.net> --- It works for the 3 file type. Thanks a lot :D Back to passed QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #37 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 178127 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178127&action=edit Bug 39115: (follow-up) Exclude Schema.pm from perltidy.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #38 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Follow-up to fix perltidy.t. In Bug 39132 comment 2, that change was planned to be included here. Maybe the plan was to eliminate the last duplication of the denylist, but at this point, that would be for a follow-up. Let's fix the tests first by fixing the 2nd denylist. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |25.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 25.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #40 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 179239 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179239&action=edit Bug 39115: get_tt_files should exclude dirs from tt, not js Silly copy/paste error -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|RM_priority |additional_work_needed Status|Pushed to main |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 --- Comment #41 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 25.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to main |Needs documenting --- Comment #42 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 38664 not in 24.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Bug 39115 depends on bug 39119, which changed state. Bug 39119 Summary: xt/js_tidy.t and xt/vue_tidy.t does not use tidy.pl https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39119 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Bug 39115 depends on bug 38664, which changed state. Bug 38664 Summary: Tidy the whole codebase https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38664 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39115 Andrew Auld <andrew.auld@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew.auld@openfifth.co.uk Status|Needs documenting |RESOLVED Resolution|--- |FIXED --- Comment #43 from Andrew Auld <andrew.auld@openfifth.co.uk> --- Architectural change - doesn't need to be documented. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org