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

Simplify image-loading-subpixel-clip.html. #22403

Merged
merged 1 commit into from Mar 25, 2020

Conversation

@chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Mar 23, 2020

Verified that it still fails with [1] applied and [2] reverted.

This may help with deflaking WPT [3]

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906
[2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698
[3] #22364

TBR:szager@chromium.org

Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752511}

Verified that it still fails with [1] applied and [2] reverted.

This may help with deflaking WPT [3]

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906
[2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698
[3] #22364

TBR:szager@chromium.org

Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752511}
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

The review process for this patch is being conducted in the Chromium project.

@KyleJu
Copy link

@KyleJu KyleJu commented Mar 24, 2020

@chrishtr Looks like this test still times out on Firefox and Chrome. Would you mind taking a second look?

Test Subtest Results Messages
/html/semantics/embedded-content/the-img-element/image-loading-subpixel-clip.html PASS: 8/10, TIMEOUT: 2/10
@chrishtr
Copy link
Contributor

@chrishtr chrishtr commented Mar 24, 2020

Will try to repro now.

@stephenmcgruer
Copy link
Contributor

@stephenmcgruer stephenmcgruer commented Mar 25, 2020

If it helps, I can reproduce on a fresh checkout of WPT via:

$ git checkout chromium-export-bb6643e9d8
$ ./wpt run --log-mach-level=info --log-mach=- --verify --verify-log-full --channel=experimental chrome html/semantics/embedded-content/the-img-element/image-loading-subpixel-clip.html 
...
 1:29.90 INFO ## Unstable results ##

 1:29.90 INFO |                                         Test                                        | Subtest |            Results            | Messages |
 1:29.90 INFO |-------------------------------------------------------------------------------------|---------|-------------------------------|----------|
 1:29.90 INFO | `/html/semantics/embedded-content/the-img-element/image-loading-subpixel-clip.html` |         | **PASS: 6/10, TIMEOUT: 4/10** |          |

Looking at the test - does target.onload fire even if the target has already loaded (if that is possible) when that script executes?

EDIT: if you want to pass a specific chrome binary in, you can pass the --binary=/path/to/chrome flag to wpt run. Note that you still need to set the 'product' as chrome, e.g. it looks like:

./wpt run --binary=/path/to/chrome --log-mach-level=info --log-mach=- --verify --verify-log-full --channel=experimental chrome html/semantics/embedded-content/the-img-element/image-loading-subpixel-clip.html

@chrishtr
Copy link
Contributor

@chrishtr chrishtr commented Mar 25, 2020

@stephenmcgruer I think you figured it out thanks. I reproduced the flakiness locally and I think
fixed it by setting the src attribute after adding the onload handler. I'll make a Chromium patch
now.

@stephenmcgruer
Copy link
Contributor

@stephenmcgruer stephenmcgruer commented Mar 25, 2020

Glad to hear it. Admin-merging this so that the following patch can be successfully exported. Note that you can check on the stabiltity checks/etc before merging a patch Chromium side. After sending it out for review look for the wpt-pr-bot to comment on it saying its been uploaded to GH (should contain a link to the PR) :).

@stephenmcgruer stephenmcgruer merged commit a82dbcb into master Mar 25, 2020
12 of 13 checks passed
12 of 13 checks passed
Community-TC (pull_request) TaskGroup: failure
Details
Azure Pipelines Build #20200323.57 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
staging.wpt.fyi - chrome[experimental] Chrome results
Details
staging.wpt.fyi - firefox[experimental] Firefox results
Details
staging.wpt.fyi - safari[experimental] Safari results
Details
wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - firefox[experimental] Firefox results
Details
wpt.fyi - safari[experimental] Safari results
Details
@stephenmcgruer stephenmcgruer deleted the chromium-export-bb6643e9d8 branch Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.