Description
Martin Renvoize (ashimema)
2020-03-06 08:25:30 UTC
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 |