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.

Sign up for GitHub

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

Jump to bottom

Add tests for the browsingContext.fragmentNavigated event #40813

Merged
merged 12 commits into from
Jul 6, 2023
Merged

Add tests for the browsingContext.fragmentNavigated event #40813

merged 12 commits into from
Jul 6, 2023

Conversation

OrKoN
Copy link
Contributor

@OrKoN OrKoN commented Jun 29, 2023 •
edited
Loading

  • ./wpt lint passes
  • tests pass with chromium-bidi
  • history API navigations
  • regular navigation
  • test_subscribe
  • test_unsubscribe
  • test_timestamp
  • test with iframes
  • test with document.write
  • test with a base tag

@OrKoN OrKoN changed the title Add tests for same-document navigations Add tests for fragmentNavigated event for same-document navigations Jun 29, 2023
@OrKoN
Copy link
Contributor Author

OrKoN commented Jun 29, 2023

It looks like we also need to check these events for regular navigations.

@whimboo
Copy link
Contributor

whimboo commented Jun 29, 2023

It looks like we also need to check these events for regular navigations.

I assume it means that you will add more tests and that a review should happen once the update has been pushed?

@OrKoN OrKoN marked this pull request as draft June 29, 2023 17:04
@OrKoN
Copy link
Contributor Author

OrKoN commented Jun 29, 2023

@whimboo if you have some other ideas for which tests we would need, feel free to comment (I will add the regular navigation tests tomorrow)

@OrKoN OrKoN changed the title Add tests for fragmentNavigated event for same-document navigations Add tests for the browsingContext.fragmentNavigated event Jun 30, 2023
@OrKoN OrKoN marked this pull request as ready for review June 30, 2023 07:20
@OrKoN
Copy link
Contributor Author

OrKoN commented Jun 30, 2023

Added a few tests (should be ready for review)

@OrKoN
Copy link
Contributor Author

OrKoN commented Jun 30, 2023

@thiagowfx PTAL

@OrKoN OrKoN requested a review from thiagowfx June 30, 2023 07:49
@OrKoN OrKoN requested a review from whimboo July 3, 2023 08:20
whimboo
whimboo requested changes Jul 3, 2023
View reviewed changes
Copy link
Contributor

@whimboo whimboo left a comment

Choose a reason for hiding this comment

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

Thanks Alex for the PR. I had a look at it and have a couple of comments that you can find inline. Not all are blockers but we should have at least the same coverage as the other existing navigation events.

webdriver/tests/bidi/browsing_context/fragment_navigated/fragment_navigated.py Show resolved Hide resolved
webdriver/tests/bidi/browsing_context/fragment_navigated/fragment_navigated.py Outdated Show resolved Hide resolved
webdriver/tests/bidi/browsing_context/fragment_navigated/fragment_navigated.py Outdated Show resolved Hide resolved
webdriver/tests/bidi/browsing_context/fragment_navigated/fragment_navigated.py Outdated Show resolved Hide resolved
webdriver/tests/bidi/browsing_context/fragment_navigated/fragment_navigated.py Show resolved Hide resolved
@thiagowfx thiagowfx mentioned this pull request Jul 3, 2023
Closed
whimboo
whimboo approved these changes Jul 5, 2023
View reviewed changes
Copy link
Contributor

@whimboo whimboo left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good to me now.

@thiagowfx mind another final review from Google's side?

@thiagowfx
Copy link
Member

@Lightning00Blade will review from the google side as he's more familiar with this than I am. I'll take another look anyway but please wait for his review before merging.

@thiagowfx
Copy link
Member

@jgraham this can be merged now.

@whimboo whimboo mentioned this pull request Jul 6, 2023
Closed
@jgraham jgraham merged commit 2665823 into web-platform-tests:master Jul 6, 2023
@OrKoN OrKoN deleted the orkon/webdriver/bidi/fragmentNavigated branch July 6, 2023 07:19
@whimboo
Copy link
Contributor

whimboo commented Jul 6, 2023

@OrKoN thanks again for writing the tests!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lightning00Blade Lightning00Blade Lightning00Blade approved these changes

@thiagowfx thiagowfx thiagowfx approved these changes

@whimboo whimboo whimboo approved these changes

@AutomatedTester AutomatedTester Awaiting requested review from AutomatedTester

@jgraham jgraham Awaiting requested review from jgraham

@jrandolf-2 jrandolf-2 Awaiting requested review from jrandolf-2

@juliandescottes juliandescottes Awaiting requested review from juliandescottes

@sadym-chromium sadym-chromium Awaiting requested review from sadym-chromium

@shs96c shs96c Awaiting requested review from shs96c

Assignees

@jgraham jgraham

Labels
webdriver-bidi
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@OrKoN @whimboo @thiagowfx @Lightning00Blade @jgraham @wpt-pr-bot