[Bug 10539] New: Remove unused unit tests in lib/KohaTest
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Bug ID: 10539 Summary: Remove unused unit tests in lib/KohaTest Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Test Suite Assignee: m.de.rooy@rijksmuseum.nl Reporter: m.de.rooy@rijksmuseum.nl As discussed on the dev list, we can remove the unit tests in lib/KohaTest. Galen suggested: [I have no objection to a patch that removes them, but it would be a niceness to look over the individual tests and see which ones can be easily ported to the current DB-dependent test framework.] Since these unit tests are not used now, they only generate unnecessary maintenance; they are regularly updated for nothing. I propose to remove them rightaway. But with Galen's suggestion in mind, I will walk through them (placed outside the Koha codebase) in the next couple of months to see if we can still retrieve some additional tests and inject them in the active db_dependent unit tests. Anyone else who wants to join that activity is welcome; you can easily retrieve these files from git. Please let me know if you do. I will open a separate report for that purpose. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 --- Comment #1 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 19389 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19389&action=edit Bug 10539: Remove unused unit tests in lib/KohaTest This patch removes all files in lib/KohaTest and the associated script t/db_dependent/database_dependent.pl. The second patch deals with a few consequences. Note that bug 10540 has been opened to save some interesting code from these unit tests and inject them in the currently used t/db_dependent suite. Test plan: See second patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 --- Comment #2 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 19390 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19390&action=edit Bug 10539: Followup for removing KohaTest unit tests Adds a line to skip testing Koha::Plugins::Base in t/00-load.t. This needs some attention. Makefile.PL: Adds always-false condition to skip section of creating file t/test-config.txt. This file is no longer used. (Next step should be to remove that code.) Removes t/Makefile and t/rewrite-config-test for the same reason. Modifies XISBN.t to remove two commented lines. Test plan: Grep for KohaTest, database_dependent, config-test, t/Makefile. Run perl Makefile.PL and include running the test suite. This may fail on t/00-valid-xml.t and t/QueryParser.t, but that is not the result of this patch. I had this result: Test Summary Report: t/00-valid-xml.t (Wstat: 1792 Tests: 381 Failed: 7) Failed tests: 10, 30-31, 169, 181, 201-202 Non-zero exit status: 7 t/QueryParser.t (Wstat: 512 Tests: 0 Failed: 0) Non-zero exit status: 2 Parse errors: No plan found in TAP output Files=92, Tests=12385, 45 wallclock secs ( 1.74 usr 0.15 sys + 26.61 cusr 1.92 csys = 30.42 CPU) Result: FAIL Failed 2/92 test programs. 7/12385 subtests failed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=10540 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 --- Comment #3 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Note: I can use some feedback on at least following two points: Can we just remove the section that we now skip with if (0 ) in Makefile.PL? Is skipping Koha::Plugins::Base necessary in 00-load.t? Could be just my problem; it seems that Jenkins does not fail here. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 --- Comment #4 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to M. de Rooy from comment #3)
Note: I can use some feedback on at least following two points:
Can we just remove the section that we now skip with if (0 ) in Makefile.PL?
Is skipping Koha::Plugins::Base necessary in 00-load.t? Could be just my problem; it seems that Jenkins does not fail here.
Talking to myself :) I remove the if (0) section now. And will not skip the Koha::Plugins::Base. If you have trouble with it too, just do the same as I did in the obsoleted patch. It seems to be my own problem, perhaps with some perl dependency.. Adjusting/rebasing now. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19389|0 |1 is obsolete| | --- Comment #5 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 19564 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19564&action=edit Bug 10539: Remove unused unit tests in lib/KohaTest This patch removes all files in lib/KohaTest and the associated script t/db_dependent/database_dependent.pl. The second patch deals with a few consequences. Note that bug 10540 has been opened to save some interesting code from these unit tests and inject them in the currently used t/db_dependent suite. Test plan: See second patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19390|0 |1 is obsolete| | --- Comment #6 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 19565 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19565&action=edit Bug 10539: Followup for removing KohaTest unit tests Makefile.PL: Section for creating file t/test-config.txt removed. Removes t/Makefile and t/rewrite-config-test too. Modifies XISBN.t to remove two commented lines. Test plan: Grep for KohaTest, database_dependent, config-test, t/Makefile. Run perl Makefile.PL and include running the test suite. This may fail on t/00-valid-xml.t and t/QueryParser.t, but that is not the result of this patch. Same for t/00-load.t with potential error on Koha::Plugins::Base. I had this result: Test Summary Report: t/00-valid-xml.t (Wstat: 1792 Tests: 381 Failed: 7) Failed tests: 10, 30-31, 169, 181, 201-202 Non-zero exit status: 7 t/QueryParser.t (Wstat: 512 Tests: 0 Failed: 0) Non-zero exit status: 2 Parse errors: No plan found in TAP output Files=92, Tests=12385, 45 wallclock secs ( 1.74 usr 0.15 sys + 26.61 cusr 1.92 csys = 30.42 CPU) Result: FAIL Failed 2/92 test programs. 7/12385 subtests failed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |10426 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |srdjan@catalyst.net.nz --- Comment #7 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- A philosophical question - changes like this, consisting of only removing unused/unwanted code, can those be self-signed, and go to QA directly? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19564|0 |1 is obsolete| | --- Comment #8 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Created attachment 20786 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20786&action=edit Bug 10539: Remove unused unit tests in lib/KohaTest This patch removes all files in lib/KohaTest and the associated script t/db_dependent/database_dependent.pl. The second patch deals with a few consequences. Note that bug 10540 has been opened to save some interesting code from these unit tests and inject them in the currently used t/db_dependent suite. Test plan: See second patch. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19565|0 |1 is obsolete| | --- Comment #9 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Created attachment 20787 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20787&action=edit Bug 10539: Followup for removing KohaTest unit tests Makefile.PL: Section for creating file t/test-config.txt removed. Removes t/Makefile and t/rewrite-config-test too. Modifies XISBN.t to remove two commented lines. Test plan: Grep for KohaTest, database_dependent, config-test, t/Makefile. Run perl Makefile.PL and include running the test suite. This may fail on t/00-valid-xml.t and t/QueryParser.t, but that is not the result of this patch. Same for t/00-load.t with potential error on Koha::Plugins::Base. I had this result: Test Summary Report: t/00-valid-xml.t (Wstat: 1792 Tests: 381 Failed: 7) Failed tests: 10, 30-31, 169, 181, 201-202 Non-zero exit status: 7 t/QueryParser.t (Wstat: 512 Tests: 0 Failed: 0) Non-zero exit status: 2 Parse errors: No plan found in TAP output Files=92, Tests=12385, 45 wallclock secs ( 1.74 usr 0.15 sys + 26.61 cusr 1.92 csys = 30.42 CPU) Result: FAIL Failed 2/92 test programs. 7/12385 subtests failed. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> All tests pass, and I find no mention of the removed test code. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |jonathan.druart@biblibre.co | |m QA Contact| |jonathan.druart@biblibre.co | |m --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- QA comment: This patch is consistent with the current work in progress on unit tests. The KohaTest is obsolete, unused, buggy, etc. The interesting code can be retrieve from the git log. Marked as Passed QA. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20786|0 |1 is obsolete| | --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 20795 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20795&action=edit Bug 10539: Remove unused unit tests in lib/KohaTest This patch removes all files in lib/KohaTest and the associated script t/db_dependent/database_dependent.pl. The second patch deals with a few consequences. Note that bug 10540 has been opened to save some interesting code from these unit tests and inject them in the currently used t/db_dependent suite. Test plan: See second patch. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20787|0 |1 is obsolete| | --- Comment #12 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 20796 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20796&action=edit Bug 10539: Followup for removing KohaTest unit tests Makefile.PL: Section for creating file t/test-config.txt removed. Removes t/Makefile and t/rewrite-config-test too. Modifies XISBN.t to remove two commented lines. Test plan: Grep for KohaTest, database_dependent, config-test, t/Makefile. Run perl Makefile.PL and include running the test suite. This may fail on t/00-valid-xml.t and t/QueryParser.t, but that is not the result of this patch. Same for t/00-load.t with potential error on Koha::Plugins::Base. I had this result: Test Summary Report: t/00-valid-xml.t (Wstat: 1792 Tests: 381 Failed: 7) Failed tests: 10, 30-31, 169, 181, 201-202 Non-zero exit status: 7 t/QueryParser.t (Wstat: 512 Tests: 0 Failed: 0) Non-zero exit status: 2 Parse errors: No plan found in TAP output Files=92, Tests=12385, 45 wallclock secs ( 1.74 usr 0.15 sys + 26.61 cusr 1.92 csys = 30.42 CPU) Result: FAIL Failed 2/92 test programs. 7/12385 subtests failed. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> All tests pass, and I find no mention of the removed test code. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gmcharlt@gmail.com --- Comment #13 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to Srdjan Jankovic from comment #7)
A philosophical question - changes like this, consisting of only removing unused/unwanted code, can those be self-signed, and go to QA directly?
A philosophical answer -- no, for two reasons. If a patch removes an unused something, having at least two pairs of eyes can help verify that (a) the something was in fact unused and (b) the something was /completely/ removed. Second, it gives folks time to jump in case they question the removal. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10539 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #14 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Marcel! I note that the RUN_DATABASE_TESTS bits still left in Makefile.PL currently stand unused, but since they provide a hook for work that Jonathan is doing, I think they should stay in. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org