Bug 17742 - Test t/db_dependent/Patrons.t can fail randomly
Summary: Test t/db_dependent/Patrons.t can fail randomly
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Chris Cormack
QA Contact: Galen Charlton
URL:
Keywords:
Depends on:
Blocks: 25551
  Show dependency treegraph
 
Reported: 2016-12-07 21:42 UTC by Josef Moravec
Modified: 2020-05-20 12:26 UTC (History)
5 users (show)

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


Attachments
Bug 17742: Fix t/db_dependent/Patrons.t (2.03 KB, patch)
2016-12-07 22:00 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17742: Fix t/db_dependent/Patrons.t (2.11 KB, patch)
2016-12-09 18:22 UTC, Chris Kirby
Details | Diff | Splinter Review
Bug 17742: Fix t/db_dependent/Patrons.t (3.18 KB, patch)
2016-12-13 07:50 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17742: Fix t/db_dependent/Patrons.t (3.10 KB, patch)
2016-12-13 10:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17742: Fix t/db_dependent/Patrons.t (3.33 KB, patch)
2016-12-27 23:28 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 17742: Fix t/db_dependent/Patrons.t (3.40 KB, patch)
2016-12-28 08:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17742: Use TestBuilder to create the library and patron category (1.40 KB, patch)
2016-12-28 08:58 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Josef Moravec 2016-12-07 21:42:23 UTC
Tests on line 75 and 81 can sometimes, the failing line is:
is( dt_from_string($b1_new->updated_on), dt_from_string, "borrowers.updated_on should have been set to now on creating" );

And the problem could more often happen on slower machines...
We are testing the date and time of updated_on, but creating the test patrons, retrieving tested patron and test itself definitely takes some (short) time... which sometimes can be on the edge of a second and you can see this:

#   Failed test 'borrowers.updated_on should have been set to now on creating'
#   at t/db_dependent/Patrons.t line 75.
#          got: '2016-12-06T20:44:25'
#     expected: '2016-12-06T20:44:26'

and also, the both tests on line 75 and 81 are the same, which is not intended I think...
Comment 1 Josef Moravec 2016-12-07 22:00:42 UTC
Created attachment 58033 [details] [review]
Bug 17742: Fix t/db_dependent/Patrons.t

Moves the getting of testing date for updated_on just after the storing
the test patron data to make the gap between generating data and now
date as short as possible

Changes test #7 to the one which was probably originally intended

Test plan
1. prove t/db_dependent/Patrons.t
2. read the diff
Comment 2 Chris Kirby 2016-12-09 18:22:58 UTC
Created attachment 58085 [details] [review]
Bug 17742: Fix t/db_dependent/Patrons.t

Moves the getting of testing date for updated_on just after the storing
the test patron data to make the gap between generating data and now
date as short as possible

Changes test #7 to the one which was probably originally intended

Test plan
1. prove t/db_dependent/Patrons.t
2. read the diff

Signed-off-by: Chris Kirby <chris.kirby@ilsleypubliclibrary.org>
Tests passed.
Comment 3 Jonathan Druart 2016-12-12 11:08:03 UTC
(In reply to Chris Kirby from comment #2)
> Changes test #7 to the one which was probably originally intended

No, the original test was good. We want to check that the updated_on value has been updated when a patron is modified.
Comment 4 Josef Moravec 2016-12-12 13:04:55 UTC
(In reply to Jonathan Druart from comment #3)
> (In reply to Chris Kirby from comment #2)
> > Changes test #7 to the one which was probably originally intended
> 
> No, the original test was good. We want to check that the updated_on value
> has been updated when a patron is modified.

But shouldn't it be tested on $b3_new?
Comment 5 Jonathan Druart 2016-12-12 13:28:27 UTC
(In reply to Josef Moravec from comment #4)
> (In reply to Jonathan Druart from comment #3)
> > (In reply to Chris Kirby from comment #2)
> > > Changes test #7 to the one which was probably originally intended
> > 
> > No, the original test was good. We want to check that the updated_on value
> > has been updated when a patron is modified.
> 
> But shouldn't it be tested on $b3_new?

Yes it should.
Comment 6 Josef Moravec 2016-12-13 07:50:18 UTC
Created attachment 58148 [details] [review]
Bug 17742: Fix t/db_dependent/Patrons.t

Moves the getting of testing date for updated_on just after the storing
the test patron data to make the gap between generating data and now
date as short as possible

Fixes test 7

Use Koha::Database instead of C4::Context->dbh

Test plan
1. prove t/db_dependent/Patrons.t
2. read the diff

Signed-off-by: Chris Kirby <chris.kirby@ilsleypubliclibrary.org>
Tests passed.
Comment 7 Josef Moravec 2016-12-13 07:57:15 UTC
(In reply to Jonathan Druart from comment #3)
> (In reply to Chris Kirby from comment #2)
> > Changes test #7 to the one which was probably originally intended
> 
> No, the original test was good. We want to check that the updated_on value
> has been updated when a patron is modified.

I change the back to original test, fix it (I hope), now it's tested on firstname, instead of surname, which is needed in later test... and maked the tests use Koha::Database instead of C4:Context->dbh;
Comment 8 Josef Moravec 2016-12-13 10:38:37 UTC
Created attachment 58150 [details] [review]
Bug 17742: Fix t/db_dependent/Patrons.t

Moves the getting of testing date for updated_on just after the storing
the test patron data to make the gap between generating data and now
date as short as possible

Fixes test 7

Use Koha::Database instead of C4::Context->dbh

Test plan
1. prove t/db_dependent/Patrons.t
2. read the diff
Comment 9 Mark Tompsett 2016-12-27 23:28:59 UTC
Created attachment 58460 [details] [review]
Bug 17742: Fix t/db_dependent/Patrons.t

Moves the getting of testing date for updated_on just after the storing
the test patron data to make the gap between generating data and now
date as short as possible

Fixes test 7

Use Koha::Database instead of C4::Context->dbh

Test plan
1. prove t/db_dependent/Patrons.t
2. read the diff

NOTE: Only minor improvement could be using test builder to
      generate the category and branch codes, rather than assume
      data exists. However, that is beyond scope of this bug.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 10 Jonathan Druart 2016-12-28 08:58:13 UTC
Created attachment 58462 [details] [review]
Bug 17742: Fix t/db_dependent/Patrons.t

Moves the getting of testing date for updated_on just after the storing
the test patron data to make the gap between generating data and now
date as short as possible

Fixes test 7

Use Koha::Database instead of C4::Context->dbh

Test plan
1. prove t/db_dependent/Patrons.t
2. read the diff

NOTE: Only minor improvement could be using test builder to
      generate the category and branch codes, rather than assume
      data exists. However, that is beyond scope of this bug.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 11 Jonathan Druart 2016-12-28 08:58:17 UTC
Created attachment 58463 [details] [review]
Bug 17742: Use TestBuilder to create the library and patron category

Moreover we do not need to remove the existing issues and patrons

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 12 Kyle M Hall 2016-12-28 13:34:31 UTC
Pushed to master for 17.05, thanks Josef, Jonathan!
Comment 13 Katrin Fischer 2017-01-04 07:27:32 UTC
These patches have been pushed to 16.11.x and will be in 16.11.02.
Comment 14 Mason James 2017-01-27 02:38:48 UTC
Pushed to 16.05.x, for 16.05.08 release