Bug 23177

Summary: Rollback cleanup in Circulation.t
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Test SuiteAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, lucas
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.02
Bug Depends on:    
Bug Blocks: 23103    
Attachments: Bug 23177: (QA follow-up) Move three subs from the middle to the top in Circulation.t
Bug 23177: (QA follow-up) Move rollback to the end
Bug 23177: (QA follow-up) Remove subtest txn_begin and rollback
Bug 23177: (QA follow-up) Move three subs from the middle to the top in Circulation.t
Bug 23177: (QA follow-up) Move rollback to the end
Bug 23177: (QA follow-up) Remove subtest txn_begin and rollback
Bug 23177: (RM follow-up) Further test clarifications
Bug 23177: [18.11.x] (QA follow-up) Move three subs from the middle to the top in Circulation.t

Description Marcel de Rooy 2019-06-21 09:05:07 UTC
Move subs to the front and rollbacks after last subtest.
Trivial edits.
Comment 1 Marcel de Rooy 2019-06-21 09:18:30 UTC
Created attachment 90880 [details] [review]
Bug 23177: (QA follow-up) Move three subs from the middle to the top in Circulation.t

Removed trailing comma for last sub too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2019-06-21 09:18:34 UTC
Created attachment 90881 [details] [review]
Bug 23177: (QA follow-up) Move rollback to the end

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2019-06-21 09:18:38 UTC
Created attachment 90882 [details] [review]
Bug 23177: (QA follow-up) Remove subtest txn_begin and rollback

This resolves:
DBIx::Class::Storage::txn_rollback(): A txn_rollback in nested transaction is ineffective! (depth 1)
Removing unneeded subtest txn's. We should probably add them everywhere or
just at global level.

Test plan:
With all three patches applied, run Circulation.t a few times.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Marcel de Rooy 2019-06-21 09:26:20 UTC
RM: Consider these three patches as QA follow-ups for 23103
Comment 5 Martin Renvoize 2019-06-21 11:32:38 UTC
Created attachment 90899 [details] [review]
Bug 23177: (QA follow-up) Move three subs from the middle to the top in Circulation.t

Removed trailing comma for last sub too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2019-06-21 11:32:41 UTC
Created attachment 90900 [details] [review]
Bug 23177: (QA follow-up) Move rollback to the end

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2019-06-21 11:32:43 UTC
Created attachment 90901 [details] [review]
Bug 23177: (QA follow-up) Remove subtest txn_begin and rollback

This resolves:
DBIx::Class::Storage::txn_rollback(): A txn_rollback in nested transaction is ineffective! (depth 1)
Removing unneeded subtest txn's. We should probably add them everywhere or
just at global level.

Test plan:
With all three patches applied, run Circulation.t a few times.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2019-06-21 11:32:46 UTC
Created attachment 90902 [details] [review]
Bug 23177: (RM follow-up) Further test clarifications

This patch makes a few minor improvements to Circulation.t
1) Adds a name to some of the scoped blocks by converting them to
   subtests.
2) Adds output messages to some tests where they were missing.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2019-06-21 11:34:01 UTC
Nice work!

Pushed to master for 19.11.00
Comment 10 Fridolin Somers 2019-07-16 06:20:17 UTC
Pushed to 19.05.x for 19.05.02
Comment 11 Lucas Gass 2019-07-25 18:12:51 UTC
cant clean apply this patchset to 18.11.x, no backport. If it is needed please help me with a rebase
Comment 12 Nick Clemens 2019-08-13 11:58:16 UTC
Created attachment 92175 [details] [review]
Bug 23177: [18.11.x] (QA follow-up) Move three subs from the middle to the top in Circulation.t

Removed trailing comma for last sub too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23177: (QA follow-up) Move rollback to the end

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23177: (QA follow-up) Remove subtest txn_begin and rollback

This resolves:
DBIx::Class::Storage::txn_rollback(): A txn_rollback in nested transaction is ineffective! (depth 1)
Removing unneeded subtest txn's. We should probably add them everywhere or
just at global level.

Test plan:
With all three patches applied, run Circulation.t a few times.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23177: (RM follow-up) Further test clarifications

This patch makes a few minor improvements to Circulation.t
1) Adds a name to some of the scoped blocks by converting them to
   subtests.
2) Adds output messages to some tests where they were missing.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23177: Rebase fixes
Comment 13 Lucas Gass 2019-08-13 20:30:30 UTC
Thanks Nick! 

backported to 18.11.x for 18.11.09