|
Description
Marcel de Rooy
2023-05-25 13:07:02 UTC
Created attachment 151679 [details] [review] Bug 33833: Use database in t/Socialdata.t Remove T::D::C. Add subtests. Remove useless parameter from get_report call. Add two additional trivial tests. Note: The module C4::SocialData looks a bit weird. Test plan: Run t/SocialData.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 151680 [details] [review] Bug 33833: Add two FIXMEs to SocialData::get_report Just comments, no test plan. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 151681 [details] [review] Bug 40002: Move to db_dependent Test plan: Run t/db_dependent/SocialData.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 151682 [details] [review] Bug 33833: Move to db_dependent Test plan: Run t/db_dependent/SocialData.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 151689 [details] [review] Bug 33833: Use database in t/Socialdata.t Remove T::D::C. Add subtests. Remove useless parameter from get_report call. Add two additional trivial tests. Note: The module C4::SocialData looks a bit weird. Test plan: Run t/SocialData.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 151690 [details] [review] Bug 33833: Add two FIXMEs to SocialData::get_report Just comments, no test plan. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 151691 [details] [review] Bug 33833: Move to db_dependent Test plan: Run t/db_dependent/SocialData.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Owen Leonard <oleonard@myacpl.org> (In reply to Owen Leonard from comment #7) > Signed-off-by: Owen Leonard <oleonard@myacpl.org> Thanks! > Use database in t/Socialdata.t
Why?
(In reply to Jonathan Druart from comment #9) > > Use database in t/Socialdata.t > > Why? They are in fact data dependent. And we can simply replace TDC by TestBuilder here. It's data dependent, not database dependent. We should actually use more Test::DBIx::Class IMO. I don't get why we should remove it from these tests. (In reply to Jonathan Druart from comment #11) > It's data dependent, not database dependent. We should actually use more > Test::DBIx::Class IMO. I don't get why we should remove it from these tests. Yeah, every test is probably data dependent :) Well, I would rather opt for removing Test::DBIx::Class. I do not see why we should use it for a few tests while we normally build the needed data with TestBuilder. (Sometimes we still access the db while perhaps thinking we dont.) Code maintenance wise easier to have one approach. TDC can oversimplify things too, and eliminate constraints that we actually do need. In this specific case not very hard to see what happens. The module is strongly connected to the associated table. I would rather use t for tests that do not need any access to the database or some mock via TDC. Test::DBIx::Class was supposed to be widely used It allows to write non DB dependent tests, and those non DB deps tests are run when we generate the packages. So they are tests than can be caught failures early, at least it was their original goal. I don't mind removing Test::DBIx::Class and rethink the real need of non DB tests, but then we discuss it, vote for it, and remove Test::DBIx::Class completely. Created attachment 155091 [details] [review] Bug 33833: Use database in t/Socialdata.t Remove T::D::C. Add subtests. Remove useless parameter from get_report call. Add two additional trivial tests. Note: The module C4::SocialData looks a bit weird. Test plan: Run t/SocialData.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 155092 [details] [review] Bug 33833: Add two FIXMEs to SocialData::get_report Just comments, no test plan. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 155093 [details] [review] Bug 33833: Move to db_dependent Test plan: Run t/db_dependent/SocialData.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 23.11. Nice work everyone, thanks! Enhancement not backported to 23.05.x |