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 wdspec test for BiDi network.addIntercept with invalid arguments #41291

Closed
wants to merge 2 commits into from
Closed

Add wdspec test for BiDi network.addIntercept with invalid arguments #41291

wants to merge 2 commits into from

Conversation

juliandescottes
Copy link
Contributor

No description provided.

@juliandescottes
Copy link
Contributor Author

cc @thiagowfx here's what I have locally, although it really on covers invalid arguments for addIntercept. I can't really test actual interception yet.

@juliandescottes juliandescottes mentioned this pull request Aug 2, 2023
Merged
@juliandescottes juliandescottes force-pushed the network-add-intercept-tests branch from 601d963 to 4d8d355 Compare August 3, 2023 11:20
@juliandescottes juliandescottes force-pushed the network-add-intercept-tests branch from 4d8d355 to f10d61c Compare August 3, 2023 11:47
@thiagowfx
Copy link
Member

The tests seem comprehensive enough to me. LGTM!

@juliandescottes juliandescottes force-pushed the network-add-intercept-tests branch from f10d61c to bb8bb32 Compare August 3, 2023 15:35
@juliandescottes
Copy link
Contributor Author

Thanks for taking a look @thiagowfx !

(just noticed that my habit of amending commits and using push -f is not great to check if review comments have been addressed, I'll stop doing that)

@juliandescottes
Copy link
Contributor Author

Thanks for the review here! We are going to land this in mozilla-central in the end, but it's almost identical to what was uploaded here.

@thiagowfx
Copy link
Member

Thanks! Could you ping here once it's landed downstream?

@juliandescottes
Copy link
Contributor Author

Thanks! Could you ping here once it's landed downstream?

Sure, should happen today.

@juliandescottes
Copy link
Contributor Author

@thiagowfx the tests for addIntercept were merged in #41492
I also landed similar tests for removeIntercept, but the sync PR still needs to be created for those.

@thiagowfx
Copy link
Member

Thanks, already pulling them in today to our repo

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

@thiagowfx thiagowfx thiagowfx approved these changes

@AutomatedTester AutomatedTester Awaiting requested review from AutomatedTester

@DanielRyanSmith DanielRyanSmith Awaiting requested review from DanielRyanSmith

@foolip foolip Awaiting requested review from foolip

@jgraham jgraham Awaiting requested review from jgraham

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

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

@shs96c shs96c Awaiting requested review from shs96c

@whimboo whimboo Awaiting requested review from whimboo

Assignees

@shs96c shs96c

Labels
infra webdriver webdriver-bidi wg-testing_browser
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@juliandescottes @thiagowfx @shs96c @wpt-pr-bot