[Bug 42152] New: xt/author/valid-templates.t should use Koha::Devel::Files to list the files to process
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 Bug ID: 42152 Summary: xt/author/valid-templates.t should use Koha::Devel::Files to list the files to process Initiative type: --- Sponsorship --- status: Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Test Suite Assignee: chris@bigballofwax.co.nz Reporter: jonathan.druart@gmail.com QA Contact: testopia@bugs.koha-community.org And also adjust the QA script (opac-bottom is not excluded from valid_template there) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|chris@bigballofwax.co.nz |jonathan.druart@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 --- Comment #1 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 197175 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197175&action=edit Bug 42152: use Koha::Devel::Files to list files to process - valid-templates.t We must use Koha::Devel::Files to list the files we process in our tests. This ensures that only the files from the git index are checked. Test plan: 0. Do not apply this patch time prove xt/author/valid-templates.t => Note the execution time and the number of files processed 1. Apply this patch, time prove xt/author/valid-templates.t => It's faster and we processed the same number of files 2. Edit a template, add a syntax error (e.g. remove a [% END %]) prove xt/author/valid-templates.t => It fails -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 --- Comment #2 from Jonathan Druart <jonathan.druart@gmail.com> --- main: real 1m27.401s user 1m34.680s This patch (without fork): real 0m31.871s user 0m25.825s This patch (with the fork): real 4m36.836s user 24m54.912s -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40449 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40449 [Bug 40449] xt/author/valid-templates.t generates warnings -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 --- Comment #3 from Owen Leonard <oleonard@myacpl.org> --- (In reply to Jonathan Druart from comment #1)
1. Apply this patch, time prove xt/author/valid-templates.t => It's faster and we processed the same number of files
Should the number of tests be the same? For me, in main it runs 875 tests. With the patch it runs 863 tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 --- Comment #4 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Owen Leonard from comment #3)
(In reply to Jonathan Druart from comment #1)
1. Apply this patch, time prove xt/author/valid-templates.t => It's faster and we processed the same number of files
Should the number of tests be the same? For me, in main it runs 875 tests. With the patch it runs 863 tests.
I was expecting that, but you are right, they are not. Here is a diff: % diff valid-templates-42152-ok.txt valid-templates-main-ok.txt # What is processed in main and not with the patch: 84a85
/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc => My version excluded both doc-head-close, main only excludes the OPAC one
109a111
/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/ill/backends/Standard/shared/shared.js => Not correct, it's a js file
118a121
/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/installer-doc-head-close.inc => Excluded in my version because of the regex '*doc-head-close.inc' in Koha::Devel::Files 252a256,257 /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/0001-Bug-42322-Hide-.patroninfo-and-search-facets-when-co.patch /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/0001-wip.patch => Incorrect, not in the git index
343a349,367
/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/accounting.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/authorities.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/enhanced_content.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/erm.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/i18n_l10n.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/interlibrary_loans.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/local_use.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/preservation.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/searching.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/staff_interface.pref /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/tools.pref 344a369 /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref => Should not be processed, not inc or tt
514a540
/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/how-to.tt => not in git index
726a753
/kohadevbox/koha/koha-tmpl/opac-tmpl/bootstrap/en/includes/ill/backends/Standard/shared/shared.js => should not be processed, js file
This is now what is processed now that was not before (basically all tt/inc that are outside of opac-tmpl and intranet-tmpl): 849,863d875 < /kohadevbox/koha/misc/cronjobs/rss/lastAcquired-1.0.tt < /kohadevbox/koha/misc/cronjobs/rss/lastAcquired-2.0.tt < /kohadevbox/koha/misc/cronjobs/rss/lastAcquired.tt < /kohadevbox/koha/misc/cronjobs/rss/longestUnseen.tt < /kohadevbox/koha/misc/cronjobs/rss/mostReserved.tt < /kohadevbox/koha/t/db_dependent/misc/translator/tt/en/sample-not-working-2.tt < /kohadevbox/koha/t/db_dependent/misc/translator/tt/en/sample.tt < /kohadevbox/koha/t/lib/plugins/Koha/Plugin/Test/inc/csrf-token.inc < /kohadevbox/koha/t/lib/plugins/Koha/Plugin/Test/inc/test.inc < /kohadevbox/koha/t/lib/plugins/Koha/Plugin/TestValuebuilder/test_valuebuilder_popup.tt < /kohadevbox/koha/t/mock_templates/intranet-tmpl/prog/en/includes/accounts.inc < /kohadevbox/koha/t/mock_templates/intranet-tmpl/prog/en/modules/about.tt < /kohadevbox/koha/t/mock_templates/intranet-tmpl/prog/fr-CA/includes/accounts.inc < /kohadevbox/koha/t/mock_templates/intranet-tmpl/prog/fr-CA/modules/about.tt < /kohadevbox/koha/t/mock_templates/opac-tmpl/bootstrap/en/modules/opac-csp.tt -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 --- Comment #5 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 197255 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197255&action=edit Bug 42152: Only exclude OPAC's doc-head-close.inc Prior to the previous patch we were testing those 2 files:
/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/installer-doc-head-close.inc
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |baptiste.wojtkowski@biblibr | |e.com --- Comment #6 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- I get 8 less tests now: on main Tests=873 Success real 2m27.800s user 2m43.268s sys 0m4.103s With patch, without broken template Tests=865 Success real 0m47.398s user 0m40.810s sys 0m2.509s With patch, with broken template Tests=865 real 0m47.769s user 0m40.878s sys 0m2.532s -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 --- Comment #7 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Baptiste Wojtkowski (bwoj) from comment #6)
I get 8 less tests now:
on main Tests=873 Success real 2m27.800s user 2m43.268s sys 0m4.103s
With patch, without broken template Tests=865 Success real 0m47.398s user 0m40.810s sys 0m2.509s
With patch, with broken template Tests=865 real 0m47.769s user 0m40.878s sys 0m2.532s
Yes it's expected, see comment 4. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197175|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197255|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 --- Comment #8 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 197384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197384&action=edit Bug 42152: use Koha::Devel::Files to list files to process - valid-templates.t We must use Koha::Devel::Files to list the files we process in our tests. This ensures that only the files from the git index are checked. Test plan: 0. Do not apply this patch time prove xt/author/valid-templates.t => Note the execution time and the number of files processed 1. Apply this patch, time prove xt/author/valid-templates.t => It's faster and we processed the same number of files 2. Edit a template, add a syntax error (e.g. remove a [% END %]) prove xt/author/valid-templates.t => It fails Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 --- Comment #9 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 197385 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197385&action=edit Bug 42152: Only exclude OPAC's doc-head-close.inc Prior to the previous patch we were testing those 2 files:
/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/installer-doc-head-close.inc
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.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=42152 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #10 from Victor Grousset/tuxayo <victor@tuxayo.net> --- After a double check of the files tested, I got the same expected deletions and additions as in Comment 4 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 --- Comment #11 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Comment on attachment 197384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197384 Bug 42152: use Koha::Devel::Files to list files to process - valid-templates.t Review of attachment 197384: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=42152&attachment=197384) ----------------------------------------------------------------- ::: xt/author/valid-templates.t @@ +49,5 @@
} else { $ncpu = Sys::CPU::cpu_count(); }
my $pm = Parallel::ForkManager->new($ncpu);
Remaining unused variable↑ Related, there is at least "my $ncpu;" and use threads; # used for parallel use Parallel::ForkManager; use Sys::CPU; Is this related to Comment 2? Did that comment mean that you tested with multiprocessing and it now was slower? (and not worth it to fix now that single thread takes 30 sec) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #12 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Victor Grousset/tuxayo from comment #11)
Is this related to Comment 2?
Yes, indeed, they are no longer needed.
Did that comment mean that you tested with multiprocessing and it now was slower? (and not worth it to fix now that single thread takes 30 sec)
Yes, it's slower when forking. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 --- Comment #13 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 202005 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202005&action=edit Bug 42152: Remove unused imports and variables Patch from commit 8cd6636 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42152 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43505 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43505 [Bug 43505] Use Koha::QA from the test suite -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org