[Koha-bugs] [Bug 35506] selenium/regressions.t is failing randomly

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35506

--- Comment #28 from Victor Grousset/tuxayo <victor at tuxayo.net> ---
(In reply to Jonathan Druart from comment #27)
> However I do not see how your approach is correct. Selenium waits for the
> page to load before starting the tests.

It's a confusing trap ^^ This is a thing that was systematically causing
confusion when I was trying to get help on a JavaScript chat.
It's not about the page to be loaded. It's about $(document).ready() to have
finished all it's content. The event Selenium waits for is the same as the one
$(document).ready() waits for to start executing IIUC. The content of
$(document).ready() callback is our stuff and there is no event to mark it's
end. The flag is a way to replace that lack.

> I would add a wait_for_element_visible on the page before selecting the th.

IIRC, at the beginning I added a sleep after clicking the "show checkouts"
button and the screenshot still showed the button pressed and not even the
message that checkouts are loading.
Stuff is just not loading and there is nothing to wait for, the click was too
early and didn't trigger the request for checkout list.

Ok here is a way to replicate what happens in the tests! :D

In checkouts.js
After `$(document).ready(function() {`
Add `wait (5000);`

So now it's possible to go hit the "show checkouts" button faster than the
callback is set. And we see the same things that the screenshot showed in the
early work on this ticket: the button stays down and checkouts never load.

> And I would not add more delay, 10 should be enough.

If there is no sign that the CI is hitting the timeout (we will see since there
was no wait_for_ajax here) yeah, let's not have those crazy high ones.
Question 1: Can we still keep the possibility of changing it? (the accessors
added by the patch) Because next time I work on flaky tests elsewhere, I'll
again need to extend the timeouts to be able to run a lot of times the rest of
the tests without hindrance from the very long XHR response times I get.
Question 2: I'm only thinking of this now: shouldn't I have not used accessors
but instead have given wait_for_ajax() an optional param to overload the
default max_retries? That would be easier to use than
- getting the old max_retries value,
- setting the new value
- after the wait, setting back the original.

Question 3: For a similar reason as to why we won't keep a high max_retries.
(we can expect the CI to not hit this need) Can we try without the sleeps
(DT_delay) before wait_for_ajax? It's to spare us unconditional sleep() (10 in
total) which is more bothering than a timeout and even on my machine that seems
to be very good at exposing flaky tests, the need for sleep $DT_delay didn't
show up.

-- 
You are receiving this mail because:
You are watching all bug changes.