mobile-dev-inc / maestro

Painless Mobile UI Automation
https://maestro.mobile.dev/
Apache License 2.0
5.88k stars 281 forks source link

Fix timeout unit in ScrollUntilVisibleCommand #2112

Closed vibin closed 1 week ago

vibin commented 4 weeks ago

Proposed changes

The timeout in ScrollUntilVisibleCommand should be interpreted as milliseconds: https://maestro.mobile.dev/api-reference/commands/scrolluntilvisible

This was broken by https://github.com/mobile-dev-inc/maestro/pull/2023.

Testing

Updated and ran tests

Issues fixed

Fixes https://github.com/mobile-dev-inc/maestro/issues/2108

Fishbowler commented 4 weeks ago

Works for me.

I ran this on main and on this branch:

appId: com.example.example
name: 'Test scrollUntilVisible timeout (#2108)'
---
- launchApp
- evalScript: ${maestro.startTime = new Date()}
- scrollUntilVisible:
    element: 'NON-EXISTENT'
    timeout: 1000
    optional: true
- evalScript: ${maestro.endTime = new Date()}
- evalScript: ${console.log(maestro.endTime - maestro.startTime)}
- assertTrue: ${maestro.endTime - maestro.startTime < 5000} # Far less than the 20000 default, but enough to allow for processing time

On main, you don't get past the scrollUntilVisible. On this branch, it passes.

Fishbowler commented 4 weeks ago

Hey @vibin - I don't suppose you'd mind adding this as e2e/workspaces/demo_app/scrollUntilVisible_timeout.yaml ?

vibin commented 1 week ago

Hi @Fishbowler - is this good to merge?