[Bug 33870] New: Remove T::D::C from Sitemapper.t
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Bug ID: 33870 Summary: Remove T::D::C from Sitemapper.t Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Test Suite Assignee: chris@bigballofwax.co.nz Reporter: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org Not that hard. And a follow-up adds a filtering parameter. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|chris@bigballofwax.co.nz |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 151875 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=151875&action=edit Bug 33870: Make $MAX in Sitemapper::Writer global This should preferably not be a lexical variable (in case we ever should call this under Plack). A global variable will also simplify testing (see next patch). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 151876 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=151876&action=edit Bug 33870: Polishing Sitemapper.t Create subtest. Remove (useless) mock on DateTime. Remove T::D::C module, add TestBuilder. Replace local slurp function by use File::Slurp. Test plan: Run t/db_dependent/Sitemapper.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 151877 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=151877&action=edit Bug 33870: Fix indentation in subtest Test plan: git diff -w HEAD~1.. t/db_dependent/Sitemapper.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |33871 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33871 [Bug 33871] Add where parameter to sitemap.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m --- Comment #4 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Hi Marcel, I'm looking at this to attempt an SO but I'm not 100% familiar with tests or Sitemapper.t. My understanding of this patch is that we're cleaning up Sitemapper.t and bringing it up to par a bit more with the most recent way of writing tests, e.g. Slurp. $schema->storage->txn_rollback; at the end also seems like something that was missing and I believe it should've always been there? I don't understand or see the need for the "FIXME" comment, if for the tests we need to make sure the only biblios that exist are the 2 biblios we're creating in the tests, isn't "Koha::Biblios->delete;" a perfectly fine way of doing it? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Pedro Amorim from comment #4)
Hi Marcel, I'm looking at this to attempt an SO but I'm not 100% familiar with tests or Sitemapper.t.
My understanding of this patch is that we're cleaning up Sitemapper.t and bringing it up to par a bit more with the most recent way of writing tests, e.g. Slurp.
$schema->storage->txn_rollback; at the end also seems like something that was missing and I believe it should've always been there? Yes
I don't understand or see the need for the "FIXME" comment, if for the tests we need to make sure the only biblios that exist are the 2 biblios we're creating in the tests, isn't "Koha::Biblios->delete;" a perfectly fine way of doing it? Yes, it works. And we do it more often in tests. But it is not the most elegant way. So maybe TODO could have been more appropriate?
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Pedro Amorim <pedro.amorim@ptfs-europe.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=33870 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #151875|0 |1 is obsolete| | --- Comment #6 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 153453 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153453&action=edit Bug 33870: Make $MAX in Sitemapper::Writer global This should preferably not be a lexical variable (in case we ever should call this under Plack). A global variable will also simplify testing (see next patch). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #151876|0 |1 is obsolete| | --- Comment #7 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 153454 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153454&action=edit Bug 33870: Polishing Sitemapper.t Create subtest. Remove (useless) mock on DateTime. Remove T::D::C module, add TestBuilder. Replace local slurp function by use File::Slurp. Test plan: Run t/db_dependent/Sitemapper.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #151877|0 |1 is obsolete| | --- Comment #8 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 153455 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153455&action=edit Bug 33870: Fix indentation in subtest Test plan: git diff -w HEAD~1.. t/db_dependent/Sitemapper.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart+koha@gmail. | |com --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- # FIXME Would be nice to remove both deletes again Koha::Checkouts->delete; Koha::Biblios->delete; We should not do that, even with a FIXME. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 --- Comment #10 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #9)
# FIXME Would be nice to remove both deletes again Koha::Checkouts->delete; Koha::Biblios->delete;
We should not do that, even with a FIXME.
They get removed in bug 33871 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153453|0 |1 is obsolete| | Attachment #153454|0 |1 is obsolete| | Attachment #153455|0 |1 is obsolete| | --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 155094 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155094&action=edit Bug 33870: Make $MAX in Sitemapper::Writer global This should preferably not be a lexical variable (in case we ever should call this under Plack). A global variable will also simplify testing (see next patch). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 155095 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155095&action=edit Bug 33870: Polishing Sitemapper.t Create subtest. Remove (useless) mock on DateTime. Remove T::D::C module, add TestBuilder. Replace local slurp function by use File::Slurp. Test plan: Run t/db_dependent/Sitemapper.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 155096 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155096&action=edit Bug 33870: Fix indentation in subtest Test plan: git diff -w HEAD~1.. t/db_dependent/Sitemapper.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> JD amended patch: tidy Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34336 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34336 [Bug 34336] Test::DBIx::Class should be removed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33870 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #15 from Fridolin Somers <fridolin.somers@biblibre.com> --- Not backported to 23.05.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org