Bug 26462 - t/db_dependent/Holds.t tests delete data unnecessarily
Summary: t/db_dependent/Holds.t tests delete data unnecessarily
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Nick Clemens (kidclamp)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-09-15 10:11 UTC by Nick Clemens (kidclamp)
Modified: 2021-06-14 21:33 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00
Circulation function:


Attachments
Bug 26462: Don't delete existing data for tests (3.40 KB, patch)
2020-09-15 10:19 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 26462: Don't delete existing data for tests (3.45 KB, patch)
2020-09-15 12:05 UTC, David Nind
Details | Diff | Splinter Review
Bug 26462: Don't delete existing data for tests (3.50 KB, patch)
2020-09-15 19:46 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2020-09-15 10:11:00 UTC
While testing on a larger database I noticed that the CanItemBeReserved / holds_per_day tests were hanging. It appears this is because they delete all holds, issues, items, biblios, and circulation rules.

The tests generate their own itemtype, rules, patrons, and libraries. There is no need to delete the existing data.
Comment 1 Nick Clemens (kidclamp) 2020-09-15 10:19:04 UTC
(In reply to Nick Clemens from comment #0)
> While testing on a larger database I noticed that the CanItemBeReserved /
> holds_per_day tests were hanging. It appears this is because they delete all
> holds, issues, items, biblios, and circulation rules.
> 
> The tests generate their own itemtype, rules, patrons, and libraries. There
> is no need to delete the existing data.

Following tests in the file follow the same pattern
Comment 2 Nick Clemens (kidclamp) 2020-09-15 10:19:40 UTC
Created attachment 110097 [details] [review]
Bug 26462: Don't delete existing data for tests

These tests generate their own items, patrons, and circ rules. Existing
system data will not affect the results of the tests. We do not need
to delete information.

You may be able to notice the difference in speed of tests depending on your system. It should
be faster after applying

To test:
1 - Before applying patch:
    prove -v t/db_dependent/Holds.t
2 - It passes
3 - Apply patch
4 - prove -v t/db_dependent/Holds.t
5 - It passes
Comment 3 David Nind 2020-09-15 12:05:43 UTC
Created attachment 110104 [details] [review]
Bug 26462: Don't delete existing data for tests

These tests generate their own items, patrons, and circ rules. Existing
system data will not affect the results of the tests. We do not need
to delete information.

You may be able to notice the difference in speed of tests depending on your system. It should
be faster after applying

To test:
1 - Before applying patch:
    prove -v t/db_dependent/Holds.t
2 - It passes
3 - Apply patch
4 - prove -v t/db_dependent/Holds.t
5 - It passes

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2020-09-15 12:07:23 UTC
Before patch applied:

  All tests successful.
  Files=1, Tests=67, 19 wallclock secs ( 0.04 usr  0.01 sys + 13.94 cusr  1.32 csys = 15.31 CPU)
  Result: PASS

After patch applied:

  All tests successful.
  Files=1, Tests=67,  9 wallclock secs ( 0.05 usr  0.01 sys +  7.99 cusr  0.67 csys =  8.72 CPU)
  Result: PASS
Comment 5 Katrin Fischer 2020-09-15 19:46:17 UTC
Created attachment 110121 [details] [review]
Bug 26462: Don't delete existing data for tests

These tests generate their own items, patrons, and circ rules. Existing
system data will not affect the results of the tests. We do not need
to delete information.

You may be able to notice the difference in speed of tests depending on your system. It should
be faster after applying

To test:
1 - Before applying patch:
    prove -v t/db_dependent/Holds.t
2 - It passes
3 - Apply patch
4 - prove -v t/db_dependent/Holds.t
5 - It passes

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Jonathan Druart 2020-10-01 09:46:02 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 7 Lucas Gass (lukeg) 2020-10-20 17:31:45 UTC
enhancement will not be backported to 20.05.x