https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35506
--- Comment #42 from Victor Grousset/tuxayo <[email protected]> --- 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. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
