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.

Sign up for GitHub

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

Remove printing/ test #23453

Closed
wants to merge 1 commit into from
Closed

Remove printing/ test #23453

wants to merge 1 commit into from

Conversation

jgraham
Copy link
Contributor

@jgraham jgraham commented May 7, 2020

This test ends up spawning a print dialog in non-Blink based browsers,
which causes an unnecessary test timeout. It also doesn't seem to match
a requirement in the HTML spec, and the description of the test doesn't
seem to match its actual behaviour.

@jgraham
Remove printing/ test
8f7a7a5 
This test ends up spawning a print dialog in non-Blink based browsers,
which causes an unnecessary test timeout. It also doesn't seem to match
a requirement in the HTML spec, and the description of the test doesn't
seem to match its actual behaviour.
@wpt-pr-bot
Copy link
Collaborator

There are no reviewers for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you!

@jgraham jgraham requested review from foolip and annevk May 7, 2020 11:00
@annevk annevk requested a review from chrishtr May 7, 2020 11:31
@annevk
Copy link
Member

annevk commented May 7, 2020

I agree with your analysis, but given we can ping the author perhaps @chrishtr can chime in.

@jgraham
Copy link
Contributor Author

jgraham commented May 7, 2020

I sent @chrishtr an email a couple of days ago, but didn't hear back. But you're right that I should also have pinged them here.

The TC failure is just pip install failing on a stability job that would otherwise have done nothing, so not a problem imo.

annevk
annevk approved these changes May 7, 2020
View reviewed changes
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I'll leave it up to you then.

@chrishtr
Copy link
Contributor

chrishtr commented May 7, 2020

I will write a Chromium CL that moves the test out of that directory and into a Chromium-specific one.

Please don't delete this test just now, it adds test coverage for Chromium.

@chrishtr
Copy link
Contributor

chrishtr commented May 7, 2020

FYI: https://chromium-review.googlesource.com/c/chromium/src/+/2187174

@chromium-wpt-export-bot chromium-wpt-export-bot mentioned this pull request May 7, 2020
Merged
chromium-wpt-export-bot pushed a commit that referenced this pull request May 7, 2020
@chrishtr @chromium-wpt-export-bot
Move a printing test out of WPT
d025edd 
This is because there is no spec text corresponding to this behavior,
and the test is failing on other browsers.

See also: #23453

Change-Id: Ia2d4f0c75c3b295e19f1ffec3e2e0d0fb1882c00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2187174
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766471}
chromium-wpt-export-bot pushed a commit that referenced this pull request May 7, 2020
@chrishtr @chromium-wpt-export-bot
Move a printing test out of WPT
928a59a 
This is because there is no spec text corresponding to this behavior,
and the test is failing on other browsers.

See also: #23453

Change-Id: Ia2d4f0c75c3b295e19f1ffec3e2e0d0fb1882c00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2187174
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766471}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request May 7, 2020
@chrishtr
Move a printing test out of WPT
09ba1b6 
This is because there is no spec text corresponding to this behavior,
and the test is failing on other browsers.

See also: web-platform-tests/wpt#23453

Change-Id: Ia2d4f0c75c3b295e19f1ffec3e2e0d0fb1882c00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2187174
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766471}
@chrishtr
Copy link
Contributor

chrishtr commented May 8, 2020

My CL landed. You can abandon this PR.

@chrishtr chrishtr closed this May 8, 2020
@stephenmcgruer
Copy link
Contributor

For the record, #23464 is the Chromium export that removed these files.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 15, 2020
@chrishtr @moz-wptsync-bot
Bug 1636230 [wpt PR 23464] - Move a printing test out of WPT, a=testonly
89afaa0 
Automatic update from web-platform-tests
Move a printing test out of WPT

This is because there is no spec text corresponding to this behavior,
and the test is failing on other browsers.

See also: web-platform-tests/wpt#23453

Change-Id: Ia2d4f0c75c3b295e19f1ffec3e2e0d0fb1882c00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2187174
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766471}

--

wpt-commits: 928a59a1f593450900c3a9f820c656b101f072c2
wpt-pr: 23464
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request May 15, 2020
@chrishtr @moz-wptsync-bot
Bug 1636230 [wpt PR 23464] - Move a printing test out of WPT, a=testonly
bd000c8 
Automatic update from web-platform-tests
Move a printing test out of WPT

This is because there is no spec text corresponding to this behavior,
and the test is failing on other browsers.

See also: web-platform-tests/wpt#23453

Change-Id: Ia2d4f0c75c3b295e19f1ffec3e2e0d0fb1882c00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2187174
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766471}

--

wpt-commits: 928a59a1f593450900c3a9f820c656b101f072c2
wpt-pr: 23464
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 20, 2020
@chrishtr @moz-wptsync-bot
Bug 1636230 [wpt PR 23464] - Move a printing test out of WPT, a=testonly
8d2b04c 
Automatic update from web-platform-tests
Move a printing test out of WPT

This is because there is no spec text corresponding to this behavior,
and the test is failing on other browsers.

See also: web-platform-tests/wpt#23453

Change-Id: Ia2d4f0c75c3b295e19f1ffec3e2e0d0fb1882c00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2187174
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766471}

--

wpt-commits: 928a59a1f593450900c3a9f820c656b101f072c2
wpt-pr: 23464
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request May 20, 2020
@chrishtr @moz-wptsync-bot
Bug 1636230 [wpt PR 23464] - Move a printing test out of WPT, a=testonly
28e6c5d 
Automatic update from web-platform-tests
Move a printing test out of WPT

This is because there is no spec text corresponding to this behavior,
and the test is failing on other browsers.

See also: web-platform-tests/wpt#23453

Change-Id: Ia2d4f0c75c3b295e19f1ffec3e2e0d0fb1882c00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2187174
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766471}

--

wpt-commits: 928a59a1f593450900c3a9f820c656b101f072c2
wpt-pr: 23464
@foolip foolip deleted the remove_printing branch April 14, 2021 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annevk annevk annevk approved these changes

@foolip foolip Awaiting requested review from foolip

@chrishtr chrishtr Awaiting requested review from chrishtr

Assignees
No one assigned
Labels
printing status:needs-reviewers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jgraham @wpt-pr-bot @annevk @chrishtr @stephenmcgruer