Issue #12421: tidy should also check .html files #12576

simartin · 2016-07-24T09:13:14Z

./mach test-tidy should also check .html files, to avoid patches containing badly formatted new test cases go unnoticed

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #12421
There are tests for these changes (./mach test-tidy failed without the changes to .hrml files)

This change is

highfive · 2016-07-24T09:13:19Z

Heads up! This PR modifies the following files:

@wafflespeanut: python/tidy/servo_tidy/tidy.py

highfive · 2016-07-24T09:13:20Z

Warning

These commits include an empty title element (<title></title>). Consider adding appropriate metadata.

notriddle · 2016-07-24T15:54:33Z

tests/html/tiny_test.html

@@ -1 +1 @@
-


Just to be clear, this one's trailing space is not deliberate, right? I can't tell...

I'm not sure; it was as such when the file was created via c6ab60d. Will try to find out.

Correction, it was created in a322542 and the trailing space existed then, too. It's fine to remove it at this point.

notriddle · 2016-07-24T15:57:55Z

tests/wpt/mozilla/tests/css/pre_with_tab.html

@@ -1 +1 @@
-<html><link rel='match' href='pre_with_tab_ref.html'><body><pre>	tab</pre></body></html>


This one should be a tab, not spaces. That's the whole point of the test.

Yes indeed, thanks! Will correct in the next iteration of the patch

wafflespeanut · 2016-07-24T16:00:31Z

Why can't we run our WPT lint in those files?

simartin · 2016-07-25T19:28:06Z

New patch version addressing all comments but the one about tests/html/tiny_test.html, that I still need to look at

jdm · 2016-08-03T19:29:31Z

@bors-servo: r+
Thanks @simartin!

bors-servo · 2016-08-03T19:29:32Z

📌 Commit e16bdae has been approved by jdm

bors-servo · 2016-08-03T23:52:23Z

⌛ Testing commit e16bdae with merge 54903bd...


        Auto merge of #12576 - simartin:issue_12421, r=jdm

Issue #12421: tidy should also check .html files ./mach test-tidy should also check .html files, to avoid patches containing badly formatted new test cases go unnoticed --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #12421 - [X] There are tests for these changes (./mach test-tidy failed without the changes to .hrml files)  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12576)

bors-servo · 2016-08-04T00:02:25Z

💔 Test failed - linux-dev

KiChjang · 2016-08-04T00:27:46Z

Fails tidy:

Checking files for tidiness...

./tests/wpt/mozilla/tests/css/box_shadow_blur_fixed_ref.html:23: no newline at EOF

./tests/wpt/mozilla/tests/css/box_shadow_blur_fixed.html:24: no newline at EOF

./tests/html/test_target_pseudoselector.html:17: tab on line

./tests/html/test_target_pseudoselector.html:18: tab on line

bors-servo · 2016-08-04T07:27:58Z

☔ The latest upstream changes (presumably #12682) made this pull request unmergeable. Please resolve the merge conflicts.

wafflespeanut · 2016-08-07T03:05:08Z

@simartin Can you fix the merge conflict? Next time, we'll ensure that this is prioritized for merging :)

Ms2ger · 2016-08-08T07:55:12Z

@bors-servo p=5

simartin · 2016-08-08T21:25:28Z

I'll rebase tomorrow and update the PR

wafflespeanut · 2016-08-09T03:01:45Z

Good to know :)


        Issue #12421: tidy should also check .html files

simartin · 2016-08-09T20:08:48Z

Rebased and new issues in tests/html/test_target_pseudoselector.html, tests/wpt/mozilla/tests/css/box_shadow_blur_fixed_ref.html and tests/wpt/mozilla/tests/css/box_shadow_blur_fixed.html fixed

jdm · 2016-08-09T20:19:38Z

@bors-servo: r+
Thanks!

bors-servo · 2016-08-09T20:19:39Z

📌 Commit 1e60c91 has been approved by jdm

bors-servo · 2016-08-09T20:19:42Z

⌛ Testing commit 1e60c91 with merge c6d9f2b...


        Auto merge of #12576 - simartin:issue_12421, r=jdm

Issue #12421: tidy should also check .html files ./mach test-tidy should also check .html files, to avoid patches containing badly formatted new test cases go unnoticed --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #12421 - [X] There are tests for these changes (./mach test-tidy failed without the changes to .hrml files)  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12576)

bors-servo · 2016-08-09T21:13:11Z

💔 Test failed - linux-rel

highfive · 2016-08-09T21:13:13Z

  ▶ Unexpected subtest result in /html/browsers/browsing-the-web/history-traversal/browsing_context_name.html:
  └ PASS [expected FAIL] Retaining window.name on history traversal

notriddle · 2016-08-09T21:24:19Z

@bors-servo retry

#12568

bors-servo · 2016-08-09T22:20:51Z

⌛ Testing commit 1e60c91 with merge 93004ce...


        Auto merge of #12576 - simartin:issue_12421, r=jdm

Issue #12421: tidy should also check .html files ./mach test-tidy should also check .html files, to avoid patches containing badly formatted new test cases go unnoticed --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #12421 - [X] There are tests for these changes (./mach test-tidy failed without the changes to .hrml files)  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12576)

bors-servo · 2016-08-09T23:18:40Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev

highfive assigned jdm Jul 24, 2016

highfive added the S-awaiting-review label Jul 24, 2016

notriddle reviewed Jul 24, 2016
View changes

simartin force-pushed the simartin:issue_12421 branch from 8a60ad4 to e16bdae Jul 25, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Aug 3, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Aug 4, 2016

KiChjang added the S-fails-tidy label Aug 4, 2016

highfive added the S-needs-rebase label Aug 4, 2016

jdm removed the S-tests-failed label Aug 4, 2016

Issue #12421: tidy should also check .html files

Loading status checks…

1e60c91

simartin force-pushed the simartin:issue_12421 branch from e16bdae to 1e60c91 Aug 9, 2016

highfive added the S-awaiting-review label Aug 9, 2016

highfive removed the S-awaiting-review label Aug 9, 2016

highfive removed the S-needs-rebase label Aug 9, 2016

highfive added the S-awaiting-merge label Aug 9, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Aug 9, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Aug 9, 2016

bors-servo merged commit 1e60c91 into servo:master Aug 9, 2016
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Aug 9, 2016

This was referenced Aug 9, 2016

tests: Speed up CSS tests by using smaller Japanese font file #11630

Closed

MSVC support for Servo, and CMake builds for native code #11756

Merged

simartin deleted the simartin:issue_12421 branch Aug 10, 2016

servo / servo

Issue #12421: tidy should also check .html files #12576

Issue #12421: tidy should also check .html files #12576

simartin commented Jul 24, 2016 •

edited by jdm

highfive commented Jul 24, 2016

highfive commented Jul 24, 2016

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

wafflespeanut commented Jul 24, 2016

simartin commented Jul 25, 2016

jdm commented Aug 3, 2016

bors-servo commented Aug 3, 2016

bors-servo commented Aug 3, 2016

bors-servo commented Aug 4, 2016

KiChjang commented Aug 4, 2016

bors-servo commented Aug 4, 2016

wafflespeanut commented Aug 7, 2016

Ms2ger commented Aug 8, 2016

simartin commented Aug 8, 2016

wafflespeanut commented Aug 9, 2016

simartin commented Aug 9, 2016

jdm commented Aug 9, 2016

bors-servo commented Aug 9, 2016

bors-servo commented Aug 9, 2016

bors-servo commented Aug 9, 2016

highfive commented Aug 9, 2016

notriddle commented Aug 9, 2016

bors-servo commented Aug 9, 2016

bors-servo commented Aug 9, 2016

		@@ -1 +1 @@
		<html><link rel='match' href='pre_with_tab_ref.html'><body><pre> tab</pre></body></html>

servo / servo

Join GitHub today

Issue #12421: tidy should also check .html files #12576

Issue #12421: tidy should also check .html files #12576

Conversation

simartin commented Jul 24, 2016 • edited by jdm

highfive commented Jul 24, 2016

highfive commented Jul 24, 2016

This comment has been minimized.

notriddle Jul 24, 2016 • edited

This comment has been minimized.

simartin Jul 25, 2016

This comment has been minimized.

jdm Aug 3, 2016

This comment has been minimized.

notriddle Jul 24, 2016 • edited

This comment has been minimized.

simartin Jul 25, 2016

wafflespeanut commented Jul 24, 2016

simartin commented Jul 25, 2016

jdm commented Aug 3, 2016

bors-servo commented Aug 3, 2016

bors-servo commented Aug 3, 2016

bors-servo commented Aug 4, 2016

KiChjang commented Aug 4, 2016

bors-servo commented Aug 4, 2016

wafflespeanut commented Aug 7, 2016

Ms2ger commented Aug 8, 2016

simartin commented Aug 8, 2016

wafflespeanut commented Aug 9, 2016

simartin commented Aug 9, 2016

jdm commented Aug 9, 2016

bors-servo commented Aug 9, 2016

bors-servo commented Aug 9, 2016

bors-servo commented Aug 9, 2016

highfive commented Aug 9, 2016

notriddle commented Aug 9, 2016

bors-servo commented Aug 9, 2016

bors-servo commented Aug 9, 2016

simartin commented Jul 24, 2016 •

edited by jdm

notriddle Jul 24, 2016 •

edited

notriddle Jul 24, 2016 •

edited