The tests can run at sub-second speeds, but add_cashup and outstanding_accountlines expect as this is a human interaction upon a cash register then we cannot end up with overlapping actions. We cannot use Time::Fake here as the timestamps are being added at the database trigger level.
Created attachment 100215 [details] [review] Bug 24817: Add sleep between db create actions We need to allow some time to pass between calls that add cash register lines to accountlines or cash_register_actions. We use database level triggers to maintain the timestamp fields, so cannot use Time::Fake, and the methods we are testing assume that, as human interactions, there will be at least a second between the last 'sale' on a cash register and the 'cashup' of that same cash register.
Nope, using sleep in tests is wrong. Why do not you force the timestamp (NOW-1h for instance) for the account lines?
(In reply to Jonathan Druart from comment #2) > Nope, using sleep in tests is wrong. > Why do not you force the timestamp (NOW-1h for instance) for the account > lines? Because I don't want to change add_cashup to accept a timestamp to be passed just to pass the test.
Created attachment 100275 [details] [review] Bug 24817: Horrible cludge date setting to remove sleep
That second patch does the trick.. but in my opinion feels much worse than using a sleep.
Created attachment 100303 [details] [review] Bug 24817: Add sleep between db create actions We need to allow some time to pass between calls that add cash register lines to accountlines or cash_register_actions. We use database level triggers to maintain the timestamp fields, so cannot use Time::Fake, and the methods we are testing assume that, as human interactions, there will be at least a second between the last 'sale' on a cash register and the 'cashup' of that same cash register. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 100304 [details] [review] Bug 24817: Horrible cludge date setting to remove sleep Signed-off-by: David Nind <david@davidnind.com>
Created attachment 100326 [details] [review] Bug 24817: Add sleep between db create actions We need to allow some time to pass between calls that add cash register lines to accountlines or cash_register_actions. We use database level triggers to maintain the timestamp fields, so cannot use Time::Fake, and the methods we are testing assume that, as human interactions, there will be at least a second between the last 'sale' on a cash register and the 'cashup' of that same cash register. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 100327 [details] [review] Bug 24817: Horrible cludge date setting to remove sleep Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I do not think that's horrible. It's basically what we really want to test.
(In reply to Jonathan Druart from comment #10) > I do not think that's horrible. It's basically what we really want to test. Thanks Jonathan.. think I'll reword the commit message on push.. I was in a down mood by the end of Friday..
Created attachment 100335 [details] [review] Bug 24817: Add sleep between db create actions We need to allow some time to pass between calls that add cash register lines to accountlines or cash_register_actions. We use database level triggers to maintain the timestamp fields, so cannot use Time::Fake, and the methods we are testing assume that, as human interactions, there will be at least a second between the last 'sale' on a cash register and the 'cashup' of that same cash register. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 100336 [details] [review] Bug 24817: Replace sleep calls with date setting in test data Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice work everyone! Pushed to master for 20.05
'Does not apply' to 19.11.x branch. not backporting