[Bug 28632] New: patrons.t fragile on slow boxes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Bug ID: 28632 Summary: patrons.t fragile on slow boxes Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com Playing some music and a Netflix movie on my (not that speedy) box, made t/db_dependent/api/v1/patrons.t fail consistently because of a 1 sec difference on the borrowers.updated_on attribute (set to current_timestamp on UPDATE). -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28585 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 122447 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122447&action=edit Bug 28632: Make patrons.t robust in terms of elapsed time between tests This patch changes the tests so they don't try to compare updated_on from many tests ago, so it is less fragile. I added a warn on the unmatched fields for convenience. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => SUCCESS: Tests pass! 2. Make your computer very slow 3. Repeat 1 => FAIL: You get things like: t/db_dependent/api/v1/patrons.t .. 3/7 # Failed test 'Returned patron from update matches expected' # at t/db_dependent/api/v1/patrons.t line 513. # Structures begin differing at: # $got->{updated_on} = '2021-06-25T14:26:20+00:00' # $expected->{updated_on} = '2021-06-25T14:26:19+00:00' # Looks like you failed 1 test of 42. 4. Apply this patch 5. Repeat 1 => SUCCESS: Tests pass again! 6. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122447|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 122460 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122460&action=edit Bug 28632: Make patrons.t robust in terms of elapsed time between tests This patch changes the tests so they don't try to compare updated_on from many tests ago, so it is less fragile. I added a warn on the unmatched fields for convenience. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => SUCCESS: Tests pass! 2. Make your computer very slow 3. Repeat 1 => FAIL: You get things like: t/db_dependent/api/v1/patrons.t .. 3/7 # Failed test 'Returned patron from update matches expected' # at t/db_dependent/api/v1/patrons.t line 513. # Structures begin differing at: # $got->{updated_on} = '2021-06-25T14:26:20+00:00' # $expected->{updated_on} = '2021-06-25T14:26:19+00:00' # Looks like you failed 1 test of 42. 4. Apply this patch 5. Repeat 1 => SUCCESS: Tests pass again! 6. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- Testing notes (koha-testing-docker): - I can normally get things to slow right down by playing YouTube, using koha-testing-docker, and then opening another browser or two (such as Google Chrome) - Managed to do this for this bug before (got tests to fail) and after (didn't manage to get it to fail) - (not very scientific!) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122460|0 |1 is obsolete| | --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 122739 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122739&action=edit Bug 28632: Make patrons.t robust in terms of elapsed time between tests This patch changes the tests so they don't try to compare updated_on from many tests ago, so it is less fragile. I added a warn on the unmatched fields for convenience. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => SUCCESS: Tests pass! 2. Make your computer very slow 3. Repeat 1 => FAIL: You get things like: t/db_dependent/api/v1/patrons.t .. 3/7 # Failed test 'Returned patron from update matches expected' # at t/db_dependent/api/v1/patrons.t line 513. # Structures begin differing at: # $got->{updated_on} = '2021-06-25T14:26:20+00:00' # $expected->{updated_on} = '2021-06-25T14:26:19+00:00' # Looks like you failed 1 test of 42. 4. Apply this patch 5. Repeat 1 => SUCCESS: Tests pass again! 6. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com QA Contact| |nick@bywatersolutions.com --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- I couldn't recreate failures but changes make sense and tests continue to pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Why a warn? And why don't you just delete $result->{update_on} then compare with is_deeply? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #6)
Why a warn? And why don't you just delete $result->{update_on} then compare with is_deeply?
What we want to test here is that the passed parameters are present on the response (and on the DB). Those parameters can change because we are basically using a Koha::Patron as a basis (there's a bug adding a new column to 'borrowers', for example) so it felt like we needed to loop through the passed parameters and see they all match with the response. Maybe we could assign the miss-matched attributes to a string, and make the test compare to empty? What do you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- We could have a t::lib::Compare module (we already have t::lib::Dates::compare), with a t::lib::Compare::hashrefs subroutine. It will deeply compare the 2 hashrefs passed in parameter. With a 'skip' option we could pass a list of keys to skip. As it does not answer your need, we may better have a 'datatype' option with a list of key to treat as dates. t::lib::Compare::hashrefs($hashref1, $hashref2, { datatype => { updated_on => 'date' } } ) We loop over the keys and when we find 'updated_on' we compare using t::lib::Compare::Dates. What do you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Tomas? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 123245 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123245&action=edit Bug 28632: Prevent api/v1/patrons.t to fail on dates comparison # Failed test 'Returned patron from update matches expected' # at t/db_dependent/api/v1/patrons.t line 537. # Structures begin differing at: # $got->{updated_on} = '2021-07-27T13:33:53+00:00' # $expected->{updated_on} = '2021-07-27T13:33:52+00:00' # Looks like you failed 1 test of 42. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I think this patch is simply doing what we want to do, Tomas do you confirm? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Passed QA --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #11)
I think this patch is simply doing what we want to do, Tomas do you confirm?
Assuming it's a "yes". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|tomascohen@gmail.com |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122739|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.11.00 released in| | Status|Passed QA |Pushed to master --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Patch pushed to master for 21.11.00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Version(s)|21.11.00 |21.11.00,21.05.03 released in| | Status|Pushed to master |Pushed to stable --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.11.00,21.05.03 |21.11.00,21.05.03,20.11.09 released in| | Status|Pushed to stable |Pushed to oldstable CC| |fridolin.somers@biblibre.co | |m --- Comment #15 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.09 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net Blocks| |25551 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25551 [Bug 25551] [OMNIBUS] Some tests are failing randomly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |Pushed to oldoldstable Version(s)|21.11.00,21.05.03,20.11.09 |21.11.00,21.05.03,20.11.09, released in| |20.05.15 --- Comment #16 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Backported: Pushed to 20.05.x branch for 20.05.15 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 wainuiwitikapark@catalyst.net.nz changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #17 from wainuiwitikapark@catalyst.net.nz --- Should this be backported to 19.11.x for 19.11.21? Or if it contains string changes, backported to 19.11.22? As we are currently in a string freeze. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 --- Comment #18 from wainuiwitikapark@catalyst.net.nz --- Should this be backported to 19.11.x for 19.11.22? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to wainuiwitikapark from comment #18)
Should this be backported to 19.11.x for 19.11.22?
If it applies, yes please. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 wainuiwitikapark@catalyst.net.nz changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.11.00,21.05.03,20.11.09, |21.11.00,21.05.03,20.11.09, released in|20.05.15 |20.05.15,19.11.22 Status|Pushed to oldoldstable |Pushed to oldoldoldstable --- Comment #20 from wainuiwitikapark@catalyst.net.nz --- Backported to 19.11.x for 19.11.22 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29884 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29884 [Bug 29884] Missing test in api/v1/patrons.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28632 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to oldoldoldstable |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org