Skip to content

Conversation

@srwei
Copy link
Contributor

@srwei srwei commented Oct 7, 2025

Spec: https://wicg.github.io/nav-speculation/prefetch.html#automated-testing
chromium-bidi implementation: GoogleChromeLabs/chromium-bidi@66d3585

Adding testdriver extension for the speculation module that is mainly to add the prefetch_status_updated event. No commands.

This event will replace the current timeout mechanism for speculation rules prefetch in the wpt prefetch corpus: https://github.com/web-platform-tests/wpt/tree/master/speculation-rules/prefetch

Follow up CLs will be raised to update the above tests to use the new event handling.

@sadym-chromium would you be able to help with rollover for chromium?

@sadym-chromium
Copy link
Contributor

Please add wdspec tests as well. Actually, I'd prefer to have wdspec tests first in a separate PR, but it's up to you.

@srwei
Copy link
Contributor Author

srwei commented Oct 7, 2025

Please add wdspec tests as well. Actually, I'd prefer to have wdspec tests first in a separate PR, but it's up to you.

Oh I thought webdriver/tests/bidi/external/speculation/prefetch_status_updated/ tests were the wdpsec tests

@srwei srwei requested a review from sadym-chromium October 8, 2025 06:53
@srwei srwei requested a review from sadym-chromium October 8, 2025 19:28
@srwei srwei requested a review from sadym-chromium October 10, 2025 17:59
await add_speculation_rules_and_link(new_tab, speculation_rules_2, prefetch_target_2)

with pytest.raises(TimeoutException):
await waiter.get_events(lambda events: len(events) >= 1, timeout=2)
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually use timeout of 0.5 when we want to make sure event didn't happen, Please use 0.5 seconds as well.

@srwei srwei requested a review from sadym-chromium October 22, 2025 12:58


@pytest.fixture
def add_speculation_rules(bidi_session):
Copy link
Contributor

Choose a reason for hiding this comment

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

I cannot see any usages of this fixture. Was it intended to be used in add_speculation_rules_and_link?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think we can probably remove the fixture actually.

expression=f"""
const script = document.createElement('script');
script.type = 'speculationrules';
script.textContent = `{rules}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: any particular reason the "`" instead of "'" was used here and in other method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The rules are usually multi line, and it seems cleaner to use backticks rather than adding escaping

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants