Skip to content

[html] Include omitted script#20223

Merged
foolip merged 3 commits intoweb-platform-tests:masterfrom
bocoup:html-include-omitted-script
Nov 14, 2019
Merged

[html] Include omitted script#20223
foolip merged 3 commits intoweb-platform-tests:masterfrom
bocoup:html-include-omitted-script

Conversation

@jugglinmike
Copy link
Copy Markdown
Contributor

The ReferenceError that's thrown due to the absence of this script can cause some browsers to report unstable results. I'm going to discuss addressing that at the harness level, but this fix is straightforward enough that I thought we should apply it regardless.

There are a handful of other tests that include test.js without template.js, but none of them actually depend on that script.

$ git grep -l test.js html/syntax/parsing | xargs grep -L template.js
html/syntax/parsing/html5lib_innerHTML_foreign-fragment.html
html/syntax/parsing/html5lib_innerHTML_math.html
html/syntax/parsing/html5lib_innerHTML_tests4.html
html/syntax/parsing/html5lib_innerHTML_tests6.html
html/syntax/parsing/html5lib_innerHTML_tests7.html
html/syntax/parsing/html5lib_innerHTML_tests_innerHTML_1.html
html/syntax/parsing/html5lib_innerHTML_webkit02.html

@jugglinmike
Copy link
Copy Markdown
Contributor Author

Turns out that file is procedurally generated. I learned this from a failing CI job, but it would be more helpful if the test material included a warning. In any event, I've updated the template, so those other tests now include an include.

Unfortunately, now the task titled "wpt-firefox-nightly-results-without-changes" is failing. This seems unrelated to the patch, so I'll push up an empty commit to trigger another trial.

Copy link
Copy Markdown
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

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

Looks sensible and CI is happy, so I'm happy.

@foolip foolip merged commit 8655c52 into web-platform-tests:master Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants