Bug 17562 - Acquisition.t is broken
Summary: Acquisition.t is broken
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 13321
Blocks: 13726
  Show dependency treegraph
 
Reported: 2016-11-07 10:26 UTC by Jonathan Druart
Modified: 2017-06-14 22:15 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 17562: Fix Acquisition.t (12.14 KB, patch)
2016-11-07 10:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17562: Fix Acquisition.t (12.21 KB, patch)
2016-11-07 11:11 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17562: Fix Acquisition.t (12.30 KB, patch)
2016-11-09 17:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-11-07 10:26:35 UTC

    
Comment 1 Jonathan Druart 2016-11-07 10:26:50 UTC
Certainly caused by bug 13321
Comment 2 Jonathan Druart 2016-11-07 10:28:58 UTC
Created attachment 57249 [details] [review]
Bug 17562: Fix Acquisition.t

I find these tests too much complicated and decided to remove them.
They depends on the number of fields in the DB and any time we change
this table we need to update the tests.
As I don't understand the tests, I prefer to simply remove them, they
don't seem very useful.
Feel free to provide a counter patch if you think we should keep them.

Test plan:
  prove t/db_dependent/Acquisition.t
should return green
Comment 3 Josef Moravec 2016-11-07 11:11:39 UTC
Created attachment 57251 [details] [review]
[SIGNED-OFF] Bug 17562: Fix Acquisition.t

I find these tests too much complicated and decided to remove them.
They depends on the number of fields in the DB and any time we change
this table we need to update the tests.
As I don't understand the tests, I prefer to simply remove them, they
don't seem very useful.
Feel free to provide a counter patch if you think we should keep them.

Test plan:
  prove t/db_dependent/Acquisition.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 4 Tomás Cohen Arazi 2016-11-09 17:32:25 UTC
Created attachment 57396 [details] [review]
Bug 17562: Fix Acquisition.t

I find these tests too much complicated and decided to remove them.
They depends on the number of fields in the DB and any time we change
this table we need to update the tests.
As I don't understand the tests, I prefer to simply remove them, they
don't seem very useful.
Feel free to provide a counter patch if you think we should keep them.

Test plan:
  prove t/db_dependent/Acquisition.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This tests should be removed as this patch does.
Comment 5 Kyle M Hall 2016-11-10 13:38:20 UTC
Pushed to master for 16.11, thanks Jonathan!
Comment 6 Mason James 2016-11-30 11:03:18 UTC
(In reply to Kyle M Hall from comment #5)
> Pushed to master for 16.11, thanks Jonathan!

blocked by DB changes, passing for 16.05