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

Jump to bottom

Move common/canvas-* files and update refs #19596

Merged
merged 1 commit into from Oct 11, 2019
Merged

Move common/canvas-* files and update refs #19596

merged 1 commit into from Oct 11, 2019

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented Oct 9, 2019

Also remove common/namespaces.js and fix affected files.

Part of #17913.

@zcorpan
Move common/canvas-* files and update refs
cf7c5bf 
Also remove common/namespaces.js and fix affected files.

Part of #17913.
@jugglinmike jugglinmike mentioned this pull request Oct 9, 2019
Open
16 tasks
@foolip
Copy link
Member

foolip commented Oct 9, 2019

This touches a lot of files, can you trigger full runs to gain and compare to master to gain confidence it doesn't break stuff?

@jugglinmike
Copy link
Contributor

@zcorpan Here's what @foolip has in mind:

  1. Push to the "triggers" branches that we created in Add triggers/* branches to support triggering full runs #18870 for this purpose
  2. Wait for the results collection to complete
  3. Use the wpt.fyi UI to select the results for your commit (cf7c5bf) and compare to the results for the commit on which it's based (819a234)

I've started things off with the triggers:

$ git push --force upstream HEAD:triggers/chrome_dev
$ git push --force upstream HEAD:triggers/firefox_nightly

I believe the results will show up here when they're ready: https://wpt.fyi/runs?sha=cf7c5bf&max-count=100, but you can use the Taskcluster status pages (as displayed in the GitHub commit history for those branches) to monitor the progress

Just know that there is some additional delay between Taskcluster's completion and the availability of results on wpt.fyi

@zcorpan
Copy link
Member Author

zcorpan commented Oct 10, 2019

Thanks @jugglinmike !

Diff for Firefox: https://wpt.fyi/results/?diff&filter=ADC&run_id=328910004&run_id=344000007

tl;dr: this change doesn't break stuff.

Of the changed tests:

https://wpt.fyi/results/2dcontext/imagebitmap/createImageBitmap-origin.sub.html?diff&filter=ADC&run_id=328910004&run_id=344000007 shows 1 difference in results, but that test appears to be flaky in Firefox according to the results history.

Outside changed tests:

Some big diffs in css animation tests, but these also seem to be caused by flakyness.
https://wpt.fyi/results/css/css-transforms/animation?diff&filter=ADC&run_id=328910004&run_id=344000007

Since only the canvas tests (in 2dcontext and html/semantics/embedded-content/the-canvas-element) used the common/canvas- files (nothing shows up in git grep "common/canvas-" on this branch), I assume that any other difference in results are due to flaky tests.

@gsnedders gsnedders merged commit 8f5a52b into master Oct 11, 2019
@gsnedders gsnedders deleted the zcorpan/mv-common-canvas branch October 11, 2019 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fserb fserb fserb approved these changes

@jugglinmike jugglinmike Awaiting requested review from jugglinmike

@AmeliaBR AmeliaBR Awaiting requested review from AmeliaBR

@annevk annevk Awaiting requested review from annevk

@deniak deniak Awaiting requested review from deniak

@domenic domenic Awaiting requested review from domenic

@foolip foolip Awaiting requested review from foolip

@gsnedders gsnedders Awaiting requested review from gsnedders

@jdm jdm Awaiting requested review from jdm

@jgraham jgraham Awaiting requested review from jgraham

@kenrussell kenrussell Awaiting requested review from kenrussell

@zqzhang zqzhang Awaiting requested review from zqzhang

Assignees

@jgraham jgraham

Labels
2dcontext assets html offscreen-canvas wg-html
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@zcorpan @foolip @jugglinmike @fserb @gsnedders @jgraham @wpt-pr-bot