Bug 38049

Summary: Admin/RecordSources_spec.ts is still failing randomly
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Test SuiteAssignee: Jonathan Druart <jonathan.druart>
Status: RESOLVED FIXED QA Contact: Pedro Amorim <pedro.amorim>
Severity: major    
Priority: P5 - low CC: david, lucas, martin.renvoize, oleonard, paul.derscheid, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00
Circulation function:
Bug Depends on: 37748, 37917    
Bug Blocks:    
Attachments: Revert "Bug 37917: Add some wait time to RecordSources_spec.ts for the intercepted request responses"
Bug 38049: Remove tooltip call from DT
Bug 38049: Remove tooltip call from DT
Bug 38049: Call .tooltip() only if needed
Revert "Bug 37917: Add some wait time to RecordSources_spec.ts for the intercepted request responses"
Bug 38049: Remove tooltip call from DT
Bug 38049: Call .tooltip() only if needed
Revert "Bug 37917: Add some wait time to RecordSources_spec.ts for the intercepted request responses"
Bug 38049: Remove tooltip call from DT
Bug 38049: Call .tooltip() only if needed

Description Jonathan Druart 2024-10-01 09:37:01 UTC
Record sources CRUD tests
    ✓ Add (3304ms)
    ✓ List (3217ms)
    1) Edit
    ✓ Delete (2965ms)


  4 passing (16s)
  1 failing

  1) Record sources CRUD tests
       Edit:
     TypeError: The following error originated from your application code, not from Cypress. It was caused by an unhandled promise rejection.

  > $(...).tooltip is not a function

When Cypress detects uncaught errors originating from your application it will automatically fail the current test.

This behavior is configurable, and you can choose to turn this off by listening to the `uncaught:exception` event.
Comment 1 Jonathan Druart 2024-10-01 09:38:40 UTC
Patch from bug 37917 didn't fix the problem.
Comment 2 Jonathan Druart 2024-10-01 09:38:56 UTC
Recreated locally on 14th run.
Comment 3 Jonathan Druart 2024-10-01 09:39:13 UTC
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();
Comment 4 Jonathan Druart 2024-10-01 09:39:40 UTC
(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?
Comment 5 Jonathan Druart 2024-10-01 10:09:20 UTC
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
Comment 6 Jonathan Druart 2024-10-01 10:09:22 UTC
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.
Comment 7 Jonathan Druart 2024-10-01 10:10:08 UTC
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();
Comment 8 Jonathan Druart 2024-10-01 10:12:54 UTC
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).
Comment 9 Jonathan Druart 2024-10-01 10:14:07 UTC
Then it would be caused by
  commit df1ec831bfc33e3ef46cce5afaef04abebc4f886
  Bug 37748: (follow-up) Move tooltip initialization to staff-global.js
Comment 10 Jonathan Druart 2024-10-01 12:17:10 UTC
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.
Comment 11 Jonathan Druart 2024-10-01 12:17:13 UTC
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.
Comment 12 Jonathan Druart 2024-10-01 12:31:20 UTC
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.
Comment 13 Jonathan Druart 2024-10-01 13:27:16 UTC
(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)
Comment 14 Jonathan Druart 2024-10-03 09:36:17 UTC
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
Comment 15 David Nind 2024-10-06 22:22:14 UTC
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>
Comment 16 David Nind 2024-10-06 22:22:17 UTC
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>
Comment 17 David Nind 2024-10-06 22:22:20 UTC
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>
Comment 18 David Nind 2024-10-06 22:24:53 UTC
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.
Comment 19 Jonathan Druart 2024-10-07 08:18:43 UTC
(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
Comment 20 David Nind 2024-10-07 17:10:36 UTC
(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.
Comment 21 Pedro Amorim 2024-10-08 08:12:07 UTC
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>
Comment 22 Pedro Amorim 2024-10-08 08:12:09 UTC
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>
Comment 23 Pedro Amorim 2024-10-08 08:12:12 UTC
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>
Comment 24 Katrin Fischer 2024-10-11 08:28:22 UTC
Thanks!
Comment 25 Katrin Fischer 2024-10-11 09:15:51 UTC
Pushed for 24.11!

Well done everyone, thank you!