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: fix tests that time out #11329

Closed
wants to merge 2 commits into from
Closed

Conversation

@zcorpan
Copy link
Member

zcorpan commented Jun 4, 2018

Part of #11269.

Part of #11269.
@zcorpan zcorpan requested a review from beaufortfrancois Jun 4, 2018
@wpt-pr-bot wpt-pr-bot requested a review from clelland Jun 4, 2018
@zcorpan zcorpan requested a review from Hexcles Jun 19, 2018
Copy link
Member

Hexcles left a comment

This generally LGTM, but I'd like confirmation from @clelland .

else
resolve(false);
});
} catch(e) {

This comment has been minimized.

Copy link
@Hexcles

Hexcles Jun 19, 2018

Member

I don't know much about feature policy or picture in picture, but this broad exception catch smells a bit. Are you trying to catch exceptions from video.requestPictureInPicture (e.g. undefined)?

This comment has been minimized.

Copy link
@zcorpan

zcorpan Jun 20, 2018

Author Member

Yeah. I suppose I can do t.step_func instead.

This comment has been minimized.

Copy link
@zcorpan

zcorpan Jun 20, 2018

Author Member

The function is sometimes called when there's no Test object. But I've changed to an if check instead.

@beaufortfrancois

This comment has been minimized.

Copy link
Contributor

beaufortfrancois commented Jun 20, 2018

Thanks for this patch.
Let me have a look.

@beaufortfrancois

This comment has been minimized.

Copy link
Contributor

beaufortfrancois commented Jun 20, 2018

I've just uploaded a patch at https://chromium-review.googlesource.com/c/chromium/src/+/1107626 that fixes the issue. It will be upstreamed soon.

chromium-wpt-export-bot pushed a commit that referenced this pull request Jun 20, 2018
This makes sure feature policy tests for Picture-in-Picture do not
time-out when Picture-in-Picture API is not available.

#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
@zcorpan

This comment has been minimized.

Copy link
Member Author

zcorpan commented Jun 20, 2018

Thanks. It looks like it doesn't have all of the changes here, though.

@Hexcles

This comment has been minimized.

Copy link
Member

Hexcles commented Jun 20, 2018

We can either land that CL and rebase-merge this PR, or land this PR and abandon that CL. Either is fine to me.

@beaufortfrancois

This comment has been minimized.

Copy link
Contributor

beaufortfrancois commented Jun 21, 2018

I think https://chromium-review.googlesource.com/c/chromium/src/+/1107626 addresses all time out issues. Please let me know why you're changing some async_test to promise_test.

chromium-wpt-export-bot pushed a commit that referenced this pull request Jun 21, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
@zcorpan

This comment has been minimized.

Copy link
Member Author

zcorpan commented Jun 21, 2018

I figured that since the tests were using promises they ought to use promise_test, but maybe it works OK either way.

chromium-wpt-export-bot pushed a commit that referenced this pull request Jun 25, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
@beaufortfrancois

This comment has been minimized.

Copy link
Contributor

beaufortfrancois commented Jun 25, 2018

I'm not sure to understand why but it looks like one test times out in https://travis-ci.org/web-platform-tests/wpt/jobs/396384904 while it doesn't for me locally. Do you folks have any idea why this may happening?

promise_test(async t => {
  const video1 = await loadVideo();
  const video2 = await loadVideo();
  return callWithTrustedClick(() => {
    const first = video1.requestPictureInPicture();
    const second = video2.requestPictureInPicture();
    return Promise.all([
      first,
      promise_rejects(t, 'NotAllowedError', second)
    ]);
  });
}, 'request Picture-in-Picture consumes user gesture');

and we have now both promises which always reject...

if (!('pictureInPictureEnabled' in document)) {
  HTMLVideoElement.prototype.requestPictureInPicture = function() {
    return Promise.reject('Picture-in-Picture API is not available');
  }
}
chromium-wpt-export-bot pushed a commit that referenced this pull request Jun 25, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
chromium-wpt-export-bot pushed a commit that referenced this pull request Jun 26, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
chromium-wpt-export-bot pushed a commit that referenced this pull request Jun 26, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
chromium-wpt-export-bot pushed a commit that referenced this pull request Jun 26, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
chromium-wpt-export-bot pushed a commit that referenced this pull request Jun 26, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
chromium-wpt-export-bot pushed a commit that referenced this pull request Jul 16, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
chromium-wpt-export-bot pushed a commit that referenced this pull request Jul 16, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
chromium-wpt-export-bot pushed a commit that referenced this pull request Jul 16, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
chromium-wpt-export-bot pushed a commit that referenced this pull request Jul 16, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

Besides, wpt/picture-in-picture/request-picture-in-picture is split
to work around a flaky timeout in WPT upstream that's related to
#10398

Note: based on #11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
aarongable pushed a commit to chromium/chromium that referenced this pull request Jul 20, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

Besides, wpt/picture-in-picture/request-picture-in-picture is split
to work around a flaky timeout in WPT upstream that's related to
web-platform-tests/wpt#10398

Note: based on web-platform-tests/wpt#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
Reviewed-on: https://chromium-review.googlesource.com/1107626
Reviewed-by: Robert Ma <robertma@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/master@{#576804}
chromium-wpt-export-bot pushed a commit that referenced this pull request Jul 20, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

Besides, wpt/picture-in-picture/request-picture-in-picture is split
to work around a flaky timeout in WPT upstream that's related to
#10398

Note: based on #11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
Reviewed-on: https://chromium-review.googlesource.com/1107626
Reviewed-by: Robert Ma <robertma@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/master@{#576804}
chromium-wpt-export-bot added a commit that referenced this pull request Jul 20, 2018
This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

Besides, wpt/picture-in-picture/request-picture-in-picture is split
to work around a flaky timeout in WPT upstream that's related to
#10398

Note: based on #11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
Reviewed-on: https://chromium-review.googlesource.com/1107626
Reviewed-by: Robert Ma <robertma@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/master@{#576804}
@Hexcles

This comment has been minimized.

Copy link
Member

Hexcles commented Jul 20, 2018

#11589 has landed and addressed all timeout on both Firefox and Chrome. This is no longer necessary, but feel free to reopen if I missed anything.

@Hexcles Hexcles closed this Jul 20, 2018
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 29, 2018
…sts that time out., a=testonly

Automatic update from web-platform-tests[Picture-in-Picture] Fix web platform tests that time out.

This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

Besides, wpt/picture-in-picture/request-picture-in-picture is split
to work around a flaky timeout in WPT upstream that's related to
web-platform-tests/wpt#10398

Note: based on web-platform-tests/wpt#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
Reviewed-on: https://chromium-review.googlesource.com/1107626
Reviewed-by: Robert Ma <robertma@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/master@{#576804}

--

wpt-commits: 38ef6aad13a38e3dec8ca2c3783f9ab0575ad356
wpt-pr: 11589
jankeromnes pushed a commit to jankeromnes/gecko that referenced this pull request Jul 30, 2018
…sts that time out., a=testonly

Automatic update from web-platform-tests[Picture-in-Picture] Fix web platform tests that time out.

This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

Besides, wpt/picture-in-picture/request-picture-in-picture is split
to work around a flaky timeout in WPT upstream that's related to
web-platform-tests/wpt#10398

Note: based on web-platform-tests/wpt#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
Reviewed-on: https://chromium-review.googlesource.com/1107626
Reviewed-by: Robert Ma <robertma@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/master@{#576804}

--

wpt-commits: 38ef6aad13a38e3dec8ca2c3783f9ab0575ad356
wpt-pr: 11589
@sideshowbarker sideshowbarker deleted the zcorpan/pip-timeout branch Nov 22, 2018
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
…sts that time out., a=testonly

Automatic update from web-platform-tests[Picture-in-Picture] Fix web platform tests that time out.

This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

Besides, wpt/picture-in-picture/request-picture-in-picture is split
to work around a flaky timeout in WPT upstream that's related to
web-platform-tests/wpt#10398

Note: based on web-platform-tests/wpt#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
Reviewed-on: https://chromium-review.googlesource.com/1107626
Reviewed-by: Robert Ma <robertmachromium.org>
Reviewed-by: Mounir Lamouri <mlamourichromium.org>
Commit-Queue: François Beaufort <beaufort.francoisgmail.com>
Cr-Commit-Position: refs/heads/master{#576804}

--

wpt-commits: 38ef6aad13a38e3dec8ca2c3783f9ab0575ad356
wpt-pr: 11589

UltraBlame original commit: 071bd4d29886a18128df2ce2dd60f4e150703838
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
…sts that time out., a=testonly

Automatic update from web-platform-tests[Picture-in-Picture] Fix web platform tests that time out.

This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

Besides, wpt/picture-in-picture/request-picture-in-picture is split
to work around a flaky timeout in WPT upstream that's related to
web-platform-tests/wpt#10398

Note: based on web-platform-tests/wpt#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
Reviewed-on: https://chromium-review.googlesource.com/1107626
Reviewed-by: Robert Ma <robertmachromium.org>
Reviewed-by: Mounir Lamouri <mlamourichromium.org>
Commit-Queue: François Beaufort <beaufort.francoisgmail.com>
Cr-Commit-Position: refs/heads/master{#576804}

--

wpt-commits: 38ef6aad13a38e3dec8ca2c3783f9ab0575ad356
wpt-pr: 11589

UltraBlame original commit: 071bd4d29886a18128df2ce2dd60f4e150703838
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
…sts that time out., a=testonly

Automatic update from web-platform-tests[Picture-in-Picture] Fix web platform tests that time out.

This makes sure web platform tests for Picture-in-Picture do not time
out when Picture-in-Picture API is not available.

Besides, wpt/picture-in-picture/request-picture-in-picture is split
to work around a flaky timeout in WPT upstream that's related to
web-platform-tests/wpt#10398

Note: based on web-platform-tests/wpt#11329

Bug: 806249
Change-Id: I6185253dfa4aae5111e87553581b8464032558d1
Reviewed-on: https://chromium-review.googlesource.com/1107626
Reviewed-by: Robert Ma <robertmachromium.org>
Reviewed-by: Mounir Lamouri <mlamourichromium.org>
Commit-Queue: François Beaufort <beaufort.francoisgmail.com>
Cr-Commit-Position: refs/heads/master{#576804}

--

wpt-commits: 38ef6aad13a38e3dec8ca2c3783f9ab0575ad356
wpt-pr: 11589

UltraBlame original commit: 071bd4d29886a18128df2ce2dd60f4e150703838
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.