https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35506 --- Comment #42 from Victor Grousset/tuxayo <victor@tuxayo.net> --- After some quick testing, the unexpected login page is also something that can happen with the 1st proposal. (In reply to Jonathan Druart from comment #41)
There is nothing else, it's just moving up some code (outside of document.ready).
I didn't have a reliable way to proofread but here it is: git show --color-moved --color-moved-ws=allow-indentation-change It's a perfect tool for this patch :)
That's really tough to draw clear conclusions. At least it's certain the alternate patch helps. My only idea to confirm it's as reliable as the 1st proposal is to do to still more runs. Then rebase my branch on top of b54da05c73 and do 1000 or 1500 runs and see if I have the same unexplained fails. I think I had the logout thing only once back when I did the 1st fix proposal.
I would push it and see how jenkins behave.
The alternate patch had such an easier time vs the 1st proposal to gain trust in it's reliability 🤯 Looking at the patch approach: in the end it's cleaner. Even if moving handler setup out of $(document).ready() looked dodgy, the automated tests should have been the first thing to fail if that was an issue. So nice intuition :) No need for ugly flag and probing for it! Coming back on previous questions plus some related: - Can we keep the wait_for_ajax calls? If I need to rerun these tests on a loop, finding again were the calls are needed means having to get a few batches of runs with failures to reintroduce them locally before being able to run the tests long enough to get the relevant failures. - And without a sleep $DT_delay. I never had a sign of wait_for_ajax returning immediately (due to XHR not yet started) and the test still failing due the request not over. So this place in Koha looks safe regarding the issue needing a sleep. So we can push it and see how jenkins behaves. - And in wait_for_ajax(), it was sleeping before checking for in progress XHR. Which is redundant with the use of sleep $DT_delay. Better that all the sleep is in one place. And that allow for cases like this test to have 0 sleep and have zero cost of having these wait_for_ajax(). But still being covered when it's needed. Whether locally of if for some reason the CI worker changes and become prone to failing due to XHR here. - Can we have wait_for_ajax taking a screenshot and printing the stack trace? I had failures that I couldn't diagnose and had to wait to happen again due to lack of screenshot or stack trace. - And keep the possibility of changing max_retries for wait_for_ajax. (this time I just made it an optional param) So it's easier to investigate flaky tests anywhere else in the suite with wait_for_ajax being fully ready to not cause irrelevant fails with this and the above change. See the second patch for the above secondary fixes and improvements in t/lib/Selenium.pm Also, added test plan. So if that's okay I think all is ready and it just needs someone else to run the test and signoff. And then even if we are both of the authors of a part of the changes, we can QA the part of the other and have this passed QA without needing someone else besides signoff. And finally have this test reliable :D -- You are receiving this mail because: You are watching all bug changes.