Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate script conditions eagerly #2056

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tokou
Copy link
Contributor

@tokou tokou commented Sep 15, 2024

Proposed changes

  • Conditions are already evaluated eagerly, meaning that as soon as a failing condition is evaluated, the flow is exited
  • However, the order currently is not ideal. We evaluate platform, visible, notVisible and then true
  • visible and notVisible take a lot of time to run when they are not satisfied, as there's a default timeout to check the hierarchy for the presence of an element
  • Moving the evaluation of true in the 2nd position makes a lot of cases run much faster

As an example, in our flows we had to do stuff like this to workaround this use case

- runFlow:
    when:
      true: ${ !output.skipLegalConsent }
    commands:
      - runFlow:
          when:
            visible:
              id: 'privacy_policy_confirm|.*button1'
          commands:
              - [...]

This saves several seconds each time (17s exactly)

Testing

  • Added an integration test that makes sure the visible does not even run if there's a false true before

Issues fixed

None

@Fishbowler Fishbowler added the enhancement New feature request or improvement of an existing feature label Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants