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

[feature-policy] Avoid timeouts in absence of API #10621

Closed

Conversation

jugglinmike
Copy link
Contributor

@jugglinmike jugglinmike commented Apr 25, 2018

Many tests written for the document.policy.allowedFeatures method
reference the API without first ensuring its availability. The method is
a new addition to the web platform, so this pattern produces a
ReferenceError in many browsers (i.e. all major browsers at the time of
this writing). Because the error occurs in the context of an iframe, the
test harness is unable to detect it, and the affected tests report an
error only following an extended delay.

Guard the reference with a try/catch block so that failures are
detected and reported immediately.


Here are the results of the command ./wpt run --include feature-policy chrome before applying this patch:

Ran 343 checks (29 tests, 314 subtests)
Expected results: 48
Unexpected results: 295
  test: 17 (14 timeout, 3 error)
  subtest: 278 (222 fail, 28 notrun, 28 timeout)

And after:

Ran 343 checks (29 tests, 314 subtests)
Expected results: 76
Unexpected results: 267
  test: 10 (7 timeout, 3 error)
  subtest: 257 (236 fail, 21 timeout)

Many tests written for the `document.policy.allowedFeatures` method
reference the API without first ensuring its availability. The method is
a new addition to the web platform, so this pattern produces a
ReferenceError in many browsers (i.e. all major browsers at the time of
this writing). Because the error occurs in the context of an iframe, the
test harness is unable to detect it, and the affected tests report an
error only following an extended delay.

Guard the reference with a `try`/`catch` block so that failures are
detected and reported immediately.
Copy link
Member

@zcorpan zcorpan left a comment

Choose a reason for hiding this comment

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

LGTM but leaving open for clelland

@jugglinmike
Copy link
Contributor Author

@clelland Would you mind taking a look?

@jugglinmike
Copy link
Contributor Author

In the two months that this pull request has been awaiting review, another contributor has submitted a patch to correct the problem: gh-11270. That patch was merged yesterday, so this is no longer necessary.

@zcorpan
Copy link
Member

zcorpan commented Jun 15, 2018

Sorry I forgot about this one.

@zcorpan zcorpan deleted the feature-policy-timeouts branch June 15, 2018 17:53
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.

3 participants