Description
Jonathan Druart
2024-10-01 09:37:01 UTC
Recreated locally on 14th run. commit 5fe1ad8ac66072ef7e06d21d0ccc48d388ecaf79 Bug 35402: Update the OPAC and staff interface to Bootstrap 5 - $(".dt_button_clear_filter, .columns_controls, .export_controls, .dt_button_configure_table").tooltip(); + // $(".dt_button_clear_filter, .columns_controls, .export_controls, .dt_button_configure_table").tooltip(); (In reply to Jonathan Druart from comment #3) > commit 5fe1ad8ac66072ef7e06d21d0ccc48d388ecaf79 > > Bug 35402: Update the OPAC and staff interface to Bootstrap 5 > > - $(".dt_button_clear_filter, .columns_controls, .export_controls, > .dt_button_configure_table").tooltip(); > + // $(".dt_button_clear_filter, .columns_controls, .export_controls, > .dt_button_configure_table").tooltip(); What's the point of this tooltip() call actually? Created attachment 172250 [details] [review] Revert "Bug 37917: Add some wait time to RecordSources_spec.ts for the intercepted request responses" This reverts commit 23b00bdc06d28e012724a37ed5052bf056026302. JD: No, really, don't wait. And it does not fix the issue initially raised on bug 37917 Created attachment 172251 [details] [review] Bug 38049: Remove tooltip call from DT Not really sure why we had this line, but Owen decided to remove it when upgrading to Bootstrap5. We had a cypress test failing because of this line $(...).tooltip is not a function Removing it should fix the problem. Still failing, so it's coming from the other occurrence: koha-tmpl/intranet-tmpl/prog/js/staff-global.js 376 $('[data-bs-toggle="tooltip"]').tooltip(); Note that I have seen it in the console of the browser as well, so it's not a cypress thing (it just highlighted the problem). Then it would be caused by commit df1ec831bfc33e3ef46cce5afaef04abebc4f886 Bug 37748: (follow-up) Move tooltip initialization to staff-global.js Created attachment 172259 [details] [review] Bug 38049: Remove tooltip call from DT Not really sure why we had this line, but Owen decided to remove it when upgrading to Bootstrap5. So let's remove them from the other files as well. Waiting for Owen to confirm this one. Created attachment 172260 [details] [review] Bug 38049: Call .tooltip() only if needed This is cheating, yes. We had a cypress test failing because of this line $(...).tooltip is not a function It's failing randomly on Admin/RecordSources_spec.ts, which does not need the tooltip. Hmmm, I was a loop with this test running, and I *think* it failed, but I lost the output... I will need to try again. (In reply to Jonathan Druart from comment #12) > Hmmm, I was a loop with this test running, and I *think* it failed, but I > lost the output... > > I will need to try again. Retried and looks ok (40 successful runs) Just got hit (main) by this on /erm/agreements, and the console was pointing to $( ".dt_button_clear_filter, .columns_controls, .export_controls, .dt_button_configure_table" ).tooltip() in KohaTable.vue Created attachment 172431 [details] [review] Revert "Bug 37917: Add some wait time to RecordSources_spec.ts for the intercepted request responses" This reverts commit 23b00bdc06d28e012724a37ed5052bf056026302. JD: No, really, don't wait. And it does not fix the issue initially raised on bug 37917 https://bugs.koha-community.org/show_bug.cgi?id=38049 Signed-off-by: David Nind <david@davidnind.com> Created attachment 172432 [details] [review] Bug 38049: Remove tooltip call from DT Not really sure why we had this line, but Owen decided to remove it when upgrading to Bootstrap5. So let's remove them from the other files as well. Waiting for Owen to confirm this one. Signed-off-by: David Nind <david@davidnind.com> Created attachment 172433 [details] [review] Bug 38049: Call .tooltip() only if needed This is cheating, yes. We had a cypress test failing because of this line $(...).tooltip is not a function It's failing randomly on Admin/RecordSources_spec.ts, which does not need the tooltip. Signed-off-by: David Nind <david@davidnind.com> Testing notes using KTD: 1. Ran tests manually: run_cypress --spec t/cypress/integration/Admin/RecordSources_spec.ts 2. Before patches, repeated 15 times before the tests failed. 3. After patches, repeated 21 times and no failures. 4. Not sure how to run a cypress test repeatedly without doing it manually - happy to run more times if someone could share how to do that. (In reply to David Nind from comment #18) > 4. Not sure how to run a cypress test repeatedly without doing it manually - > happy to run more times if someone could share how to do that. I am doing this (run 300x and stop on error): % for i in {1..300}; do echo "loop $i"; yarn cypress run --spec t/cypress/integration/Admin/RecordSources_spec.ts ; if [ "$?" = "1" ]; then break; fi; done (In reply to Jonathan Druart from comment #19) > (In reply to David Nind from comment #18) > > 4. Not sure how to run a cypress test repeatedly without doing it manually - > > happy to run more times if someone could share how to do that. > > I am doing this (run 300x and stop on error): > % for i in {1..300}; do echo "loop $i"; yarn cypress run --spec > t/cypress/integration/Admin/RecordSources_spec.ts ; if [ "$?" = "1" ]; then > break; fi; done Thanks Jonathan! Have run with the patches applied 300 times, and no test failures. Created attachment 172510 [details] [review] Revert "Bug 37917: Add some wait time to RecordSources_spec.ts for the intercepted request responses" This reverts commit 23b00bdc06d28e012724a37ed5052bf056026302. JD: No, really, don't wait. And it does not fix the issue initially raised on bug 37917 https://bugs.koha-community.org/show_bug.cgi?id=38049 Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 172511 [details] [review] Bug 38049: Remove tooltip call from DT Not really sure why we had this line, but Owen decided to remove it when upgrading to Bootstrap5. So let's remove them from the other files as well. Waiting for Owen to confirm this one. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 172512 [details] [review] Bug 38049: Call .tooltip() only if needed This is cheating, yes. We had a cypress test failing because of this line $(...).tooltip is not a function It's failing randomly on Admin/RecordSources_spec.ts, which does not need the tooltip. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Thanks! Pushed for 24.11! Well done everyone, thank you! |