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

Multicol width tests #11802

Merged

Conversation

rachelandrew
Copy link
Contributor

A set of cleaned up tests for multicol:

  1. Added asset where it was missing.
  2. Added a description to the page on tests to explain what a pass was.
  3. Removed CSS that wasn't required (orphans and widows properties)
  4. Tidied up multicol-width-003 as discussed in Document what tests in the multicolum test suite are about #7470
  5. Changed multicol-width-count-002 so that it fails if column-count is ignored and column-width used otherwise it appears to test the same thing as multicol-width-count-001

@gsnedders
Copy link
Member

Is there any reason this hasn't landed?

@rachelandrew rachelandrew merged commit f50337b into web-platform-tests:master Sep 3, 2018
@rachelandrew rachelandrew deleted the multicol-width-tests branch September 3, 2018 12:22
@mstensho
Copy link
Contributor

mstensho commented Sep 3, 2018

There's a mistake in this change; got automatically reported in Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=880084

I'll fix it shortly (via the Chromium repo). But my question is: Aren't automatic tests run before landing pull requests?

@gsnedders
Copy link
Member

@mstensho we don't currently do any differential comparison between how browsers pass/fail before/after a change (that's #7475), mostly because we don't store the results in the Travis environment anywhere to do a comparison against the merge base, and we don't even do the stability checking properly for reference changes (#5319).

@mstensho
Copy link
Contributor

mstensho commented Sep 3, 2018

@gsnedders That's a pity. Such mistakes are so easy to make, and could be so easy to detect as well.

Landing a fix now: https://chromium-review.googlesource.com/c/chromium/src/+/1203250
That's pull request #12815

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.

4 participants