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

Initial Input module implementation #784

Merged
merged 1 commit into from
May 26, 2023
Merged

Initial Input module implementation #784

merged 1 commit into from
May 26, 2023

Conversation

jrandolf-2
Copy link
Contributor

@jrandolf-2 jrandolf-2 commented May 26, 2023

Notes to the reviewer:

Bug: #749

@jrandolf-2 jrandolf-2 requested a review from OrKoN May 26, 2023 07:09
@jrandolf-2 jrandolf-2 mentioned this pull request May 26, 2023
29 tasks
Base automatically changed from input-module-4 to main May 26, 2023 08:07
@OrKoN OrKoN requested a review from sadym-chromium May 26, 2023 09:15
@jrandolf-2 jrandolf-2 force-pushed the input-module-5 branch 2 times, most recently from 480ffc5 to 0f41670 Compare May 26, 2023 09:30
@jrandolf-2 jrandolf-2 force-pushed the input-module-5 branch 2 times, most recently from 6b94d5e to e1c1b2b Compare May 26, 2023 09:35
@jrandolf-2 jrandolf-2 enabled auto-merge (squash) May 26, 2023 09:35
@OrKoN
Copy link
Collaborator

OrKoN commented May 26, 2023

Do we need to update some WPT expectations to see if the WPT tests pass? @sadym-chromium

tests/test_input.py Outdated Show resolved Hide resolved
@jrandolf-2 jrandolf-2 force-pushed the input-module-5 branch 2 times, most recently from eb0d1d3 to bb35958 Compare May 26, 2023 09:46
@jrandolf-2
Copy link
Contributor Author

@OrKoN Yea, we should update the WPT tests. I've added appended the headless tests, but should we do the others as well?

Copy link
Collaborator

@OrKoN OrKoN left a comment

Choose a reason for hiding this comment

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

Code LGTM but I will leave the review of WPT test expectations/e2e to Maksim since I am not very familiar with the setup.

Copy link
Collaborator

@sadym-chromium sadym-chromium left a comment

Choose a reason for hiding this comment

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

LGTM after some comments will be added

@jrandolf-2 jrandolf-2 merged commit 1fa10ea into main May 26, 2023
@jrandolf-2 jrandolf-2 deleted the input-module-5 branch May 26, 2023 10:41

# TODO(jrandolf): Investigate mouse wheel flakiness. The deltaY sometimes
# doubles and other times not. It is also not independent of the mouse.
# @pytest.mark.asyncio
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use @pytest.mark.skip(reason="TODO: Implement") instead of submitting commented out code

from anys import ANY_STR
from test_helpers import execute_command, goto_url

SCRIPT_HTML = 'data:text/html,' + """
Copy link
Contributor

Choose a reason for hiding this comment

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

Define the script without data:text/html. Then, in the test function, add the html fixture and use it as html(SCRIPT)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants