ui5-community / wdi5

official UI5 end-to-end test framework for UI5 web-apps. wdi5 = Webdriver.IO + UI5 Test API
https://ui5-community.github.io/wdi5/
Apache License 2.0
102 stars 43 forks source link

browser.asControl({timeout ... has no effect #392

Closed nair-sumesh closed 1 year ago

nair-sumesh commented 1 year ago

Describe the bug For a slow responding systems where the page takes too much time to respond, the timeout feature of the browser.asControl has no affect.

To Reproduce Not possible to reproduce on every system.

Expected behavior The timeout parameter should respect the wait for the element to load.

Logs/Console Output

Code

     await browser.asControl({
      selector: {
        controlType: 'sap.ushell.ui.launchpad.AnchorItem',
        properties: { "title":"Administrator" }
      },
      timeout: 90000,
      forceSelect: true
    })

Output image

Screenshots if applicable, add screenshots to help explain your problem.

Runtime Env (please complete the following information):

nair-sumesh commented 1 year ago

Further Observation

sap.ui.test.autowaiter._autoWaiter

Config being passed to extendConfig has no defined name image

Available list of waiters are as follows image

As per my analysis, the timeout is not added to any waiters.

dominikfeininger commented 1 year ago

Hi @nair-sumesh

in https://github.com/ui5-community/wdi5/blob/main/client-side-js/getControl.js https://sapui5.hana.ondemand.com/sdk/#/api/sap.ui.test.RecordReplay%23methods/sap.ui.test.RecordReplay.waitForUI5 to apply the custom timeout.

which was enhanced here: https://github.com/ui5-community/wdi5/blob/78091e884bdd6b5cb1972dd28c6ea40b0ac948a3/client-side-js/injectUI5.js#L51

I found some occurrences of waitForUI5 where the enhancement was not used (_getAggregation, _navTo). Would like to clarify this first @edvardas-kireilis @vobu

github-actions[bot] commented 1 year ago

hey 👋 - silence for 30 days 🤐 ... anybody? 😀

vobu commented 1 year ago

ping, still valid

Siolto commented 1 year ago

After having a hard time debugging this, I may have found the Bug:

It is not our custom timeout on the selector but the synchronization with the ui5 framework through sap/ui/test/autowaiter/_autoWaiterAsync. That's why I want to bring @hmanchev in the loop.

It seems like the sap/ui/test/autowaiter/_autoWaiterAsync is not feature complete or a little bit flaky with some requests. For example all requests via fetch are not recognized and will not be awaited till they are resolved/rejected.

Also the request fired via XMLHttpRequest which are not specifically marked as async will be also not correctly tracked.

Further investigation is needed to really find the root of this problem.

hmanchev commented 1 year ago

Hi colleagues,

OPA experts are notified about this potential bug and we are waiting for their feedback. Please stay tuned.

BR, Hristo

kineticjs commented 1 year ago

Hi @nair-sumesh, @dominikfeininger ,

Can we please clarify the expected behavior again: After checking the code, I see that the timeout parameter of sap.ui.test.RecordReplay.waitForUI5 has the following role: prevents the waitForUI5 function from waiting 'forever' i.e. stops the waiting after reaching the given timeout value (even if the app still has pending/unfinished tasks)

I'm linking a snippet that demonstrates it:

the snippet schedules a task to be executed after 900ms, but at the same time sets a wait timeout of 800ms => the waiting stops before the task is done

Steps: -- open https://jsbin.com/kigotohuko/1/edit?html,js,output -- press the "waitforUI5" button => an alert "Polling stopped because the timeout of 800 milliseconds has been reached but there is still pending asynchronous work." is displayed BEFORE the alert with "async task is done"

Locally I also simulated the same setup but with a long-running task (>90000ms) with the same result

So,@nair-sumesh: if we are on the same page, did you mean that in your setup the waitForUI5 kept waiting even after the custom timeout of 90000 passed?

kineticjs commented 1 year ago

A side note regarding the observation of @nair-sumesh that the timeout value was not passed to the individual waiters. It looked like bug to me too initially, however I checked that it does not affect the processing since the individual waiters do not need to know that 'timeout' value. The [custom] timeout is handled on a different level, see: customTimeout so it is still applied

Siolto commented 1 year ago

Hi @kineticjs

thanks for looking into this issue. Yes in your example everything is working as expected. But as already written above, it is not about the setTimeout but more about fetch or xmlHttpRequest requests which are not correctly covered via the waitForUI5. Maybe there are even more cases that are not covered by the autoWaiter but these are the two examples I could find.

I have a minimal reproducible example for you with a running express server:

For the xmlHttpRequest I may have found the line where you could start debugging. It seems like there is the bug.

For the fetch I am pretty sure this should be handled via _promiseWaiter.js but there is to much 🪄 going on to give you a concrete guidance here.

I hope my example helps you get to the root of this problem.

Regards Simon

kineticjs commented 1 year ago

Hi @Siolto,

Thank you the samples that clearly show cases not covered by the auto-waiter. I'll request internally to plan a research why these were not covered so far and discuss their inclusion.

I posted my earlier question because these two cases (fetch and sync xmlHttpRequest) are examples where the auto-waiter does not wait at all (because it does not even detect them), while the original issue was supposedly about the auto-waiter waiting too long (because @nair-sumesh mentioned the auto-waiter did not respect the custom timeout)

This is why I wanted @nair-sumesh to clarify again the original issue (waiting too long OR not waiting enough time).

nair-sumesh commented 1 year ago

Hi @Siolto,

Thank you the samples that clearly show cases not covered by the auto-waiter. I'll request internally to plan a research why these were not covered so far and discuss their inclusion.

I posted my earlier question because these two cases (fetch and sync xmlHttpRequest) are examples where the auto-waiter does not wait at all (because it does not even detect them), while the original issue was supposedly about the auto-waiter waiting too long (because @nair-sumesh mentioned the auto-waiter did not respect the custom timeout)

This is why I wanted @nair-sumesh to clarify again the original issue (waiting too long OR not waiting enough time).

@kineticjs: It's the case of not waiting for enough time. As mentioned in this thread, this happens only for few UI5 controls.
One of the cases, when the automation does not wait, is for the menu items or the tiles on the welcome page of FLP.
As you can see from my original example, the auto-waiter is not waiting for at least 90 seconds for the menu 'Administrator' to appear, before concluding to the final state - 'No dom element found using the control selector'.

Regards, Sumesh Nair

kineticjs commented 1 year ago

Hi @nair-sumesh,

"the auto-waiter is not waiting for at least 90 seconds for the menu 'Administrator' to appear"

I see... I re-checked that the timeout parameter does not instruct the auto-waiter how long to wait. The auto-waiter waits until it no longer detects ongoing async operations in the app, If those operations complete earlier than the timeout, the auto-waiter will stop waiting. Only if the operations take longer than the timeout, the auto-waiter will stop (because the role of the timeout is only to prevent the auto-waiter become stuck waiting...)

So @Siolto suggested that maybe the auto-waiter exits earlier because it cannot detect some ongoing async operations in the app, because the auto-waiter does not detect requests that use the fetch native API or xmlHttpRequest without async set to true @nair-sumesh, do you know if your app initiates such requests? I searched for usages of the fetch native API in the Core UI5 framework and couldn't find any (there is a centralized sap/base/util/fetch module that uses XMLHttpRequest underneath). The sync requests are considered legacy so I expect they are also no longer in use...

In all cases I'd suggest that we need to reproduce the actual issue in question (if we can get access to the app) to be certain we are tracing the issue in the app and not another potential issue...

kineticjs commented 1 year ago

After discussion we planned to enhance the auto-waiter with support for fetch and sync xmlHttpRequest. Thanks again @Siolto for taking the time to compose the clear samples. We will update when the new implementation is available.

kineticjs commented 1 year ago

Hi @nair-sumesh,

Just as a side note, I see you are using UI5 version 1.106.1 which does not contain a recent enhancement to the auto-waiter https://github.com/SAP/openui5/commit/6b478630a249a8311ab1e77fe29efb7111ac55da that enables it to track ongoing animations. It was included in 1.107 and all later UI5 versions. Given that 1.106 is no longer listed as supported (I'm checking against the list at https://sapui5.hana.ondemand.com/versionoverview.html) you could (if you want) from now update to 1.108 or any later supported version while we are working on the remaining discussed enhancements (tracking of fetch and XMLHttpRequest without async set to true)

github-actions[bot] commented 1 year ago

hey 👋 - silence for 30 days 🤐 ... anybody? 😀

nair-sumesh commented 1 year ago

👀

nair-sumesh commented 1 year ago

👀

github-actions[bot] commented 1 year ago

hey 👋 - silence for 30 days 🤐 ... anybody? 😀

vobu commented 1 year ago

hey @nair-sumesh , the latest version should contain a fix for that. could you please test-drive? thanks!

nair-sumesh commented 1 year ago

blocker #473

github-actions[bot] commented 1 year ago

hey 👋 - silence for 30 days 🤐 ... anybody? 😀

github-actions[bot] commented 1 year ago

closed 📴 because silencio 🤫 since an additional 14 days after staleness 📠