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

Make sure that Actions.perform is always passed a list #14345

Merged
merged 10 commits into from
Dec 10, 2018

Conversation

LanWei22
Copy link
Contributor

@LanWei22 LanWei22 commented Dec 3, 2018

Fixes #14343
When calling action_sequence.send_actions, we should pass a list not dictionary, so Actions.perform will get a list.

@wpt-pr-bot wpt-pr-bot requested a review from jgraham December 3, 2018 20:38
@NavidZ
Copy link
Member

NavidZ commented Dec 3, 2018

You need to move base.py to the right address (tools/wptrunner/wptrunner/executors/base.py) to show only the right changes.

@NavidZ NavidZ requested a review from gsnedders December 3, 2018 21:23
@wpt-pr-bot wpt-pr-bot added the wptrunner The automated test runner, commonly called through ./wpt run label Dec 3, 2018
@JohnChen0
Copy link
Contributor

While this change fixes the test on Chrome and Safari, it breaks the test on Firefox. The reason is Firefox uses a different executor, tools/wptrunner/wptrunner/executors/executormarionette.py, which expects the actions to be specified in the previous format.

To fix the test on Chrome without breaking Firefox, I think the right place is in tools/wptrunner/wptrunner/executors/executorwebdriver.py:

class WebDriverActionSequenceProtocolPart(ActionSequenceProtocolPart):
    ...
    def send_actions(self, actions):
        self.webdriver.actions.perform(actions['actions'])

Copy link
Contributor

@jgraham jgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per @JohnChen0 's comment

@LanWei22
Copy link
Contributor Author

LanWei22 commented Dec 4, 2018

Thank @JohnChen0 for your comment, I have made the change.

Copy link
Contributor Author

@LanWei22 LanWei22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have submitted the change, please take a look again, thank you.

@LanWei22
Copy link
Contributor Author

LanWei22 commented Dec 7, 2018

@jgraham I fixed the check failure, could you please take a look, thank you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra wptrunner The automated test runner, commonly called through ./wpt run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants