Bug 154996

Summary: Update template element tests
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: Tools / TestsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, dbates, lforschler
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 143519    
Attachments:
Description Flags
Updates the tests
none
Updated for ToT
none
Reimports tests dbates: review+

Description Ryosuke Niwa 2016-03-03 17:01:58 PST
We have an updated test suite in our repo.
Comment 1 Ryosuke Niwa 2016-03-03 17:04:35 PST
Created attachment 272801 [details]
Updates the tests
Comment 2 Ryosuke Niwa 2016-03-03 17:20:13 PST
Created attachment 272804 [details]
Updated for ToT
Comment 3 Daniel Bates 2016-03-18 16:01:53 PDT
Comment on attachment 272804 [details]
Updated for ToT

View in context: https://bugs.webkit.org/attachment.cgi?id=272804&action=review

OK.

> LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-template-element/template-element/template-as-a-descendant-expected.txt:7
> -PASS Template element as a descendant of the FRAMESET element. Template element is created by innerHTML 
> +FAIL Template element as a descendant of the FRAMESET element. Template element is created by innerHTML assert_equals: Template element should not be allowed as a descendant of the FRAMESET element expected null but got Element node <template>some text</template>
>  PASS Template element as an indirect descendant of the BODY element. Template element is created by innerHTML 
>  PASS Template element as an indirect descendant of the HEAD element. Template element is created by innerHTML 
> -PASS Template element as an indirect descendant of the FRAMESET element. Template element is created by innerHTML 
> +FAIL Template element as a descendant of the FRAMESET element. Template element is created by innerHTML assert_equals: Template element should not be allowed as indirect descendant of the FRAMESET element expected null but got Element node <template>some text</template>

:( You may want consider filing a bug to track fixing this (if we do not have one already).
Comment 4 Daniel Bates 2016-03-18 16:04:48 PDT
Comment on attachment 272804 [details]
Updated for ToT

View in context: https://bugs.webkit.org/attachment.cgi?id=272804&action=review

> LayoutTests/imported/w3c/ChangeLog:9
> +        Reimported W3C tests for template elements. Also moved the parser tests from html-templates to web-platform-tests/html/syntax
> +        to match the upstream directory structure.

Please include the SHA of the W3C Platform Tests repository for the revision these tests represent in this description.
Comment 5 Ryosuke Niwa 2016-03-18 17:07:04 PDT
(In reply to comment #3)
> Comment on attachment 272804 [details]
> Updated for ToT
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272804&action=review
> 
> OK.
> 
> > LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-template-element/template-element/template-as-a-descendant-expected.txt:7
> > -PASS Template element as a descendant of the FRAMESET element. Template element is created by innerHTML 
> > +FAIL Template element as a descendant of the FRAMESET element. Template element is created by innerHTML assert_equals: Template element should not be allowed as a descendant of the FRAMESET element expected null but got Element node <template>some text</template>
> >  PASS Template element as an indirect descendant of the BODY element. Template element is created by innerHTML 
> >  PASS Template element as an indirect descendant of the HEAD element. Template element is created by innerHTML 
> > -PASS Template element as an indirect descendant of the FRAMESET element. Template element is created by innerHTML 
> > +FAIL Template element as a descendant of the FRAMESET element. Template element is created by innerHTML assert_equals: Template element should not be allowed as indirect descendant of the FRAMESET element expected null but got Element node <template>some text</template>
> 
> :( You may want consider filing a bug to track fixing this (if we do not
> have one already).

I'm fixing it in https://bugs.webkit.org/show_bug.cgi?id=143519.  Let me upload a new patch since I've fixed a few more tests upstream while this patch was waiting in the review queue.
Comment 6 Ryosuke Niwa 2016-04-25 20:58:47 PDT
Created attachment 277324 [details]
Reimports tests
Comment 7 Daniel Bates 2016-04-25 21:06:52 PDT
Comment on attachment 277324 [details]
Reimports tests

View in context: https://bugs.webkit.org/attachment.cgi?id=277324&action=review

I know that the w3c-import.log files are generated by running import-w3c-tests. We should fix the text in these files to read well.

> LayoutTests/imported/w3c/web-platform-tests/html/syntax/parsing/template/additions-to-foster-parenting/w3c-import.log:2
> +Do NOT modify these tests directly in Webkit.

Webkit => WebKit

> LayoutTests/imported/w3c/web-platform-tests/html/syntax/parsing/template/additions-to-foster-parenting/w3c-import.log:7
> +Then run the Tools/Scripts/import-w3c-tests in Webkit to reimport

Webkit => WebKit

Also missing a period at the end of this sentence.

> LayoutTests/imported/w3c/web-platform-tests/html/syntax/parsing/template/additions-to-foster-parenting/w3c-import.log:9
> +Do NOT modify or remove this file

Missing a period at the end of this sentence.
Comment 8 Ryosuke Niwa 2016-04-25 21:20:22 PDT
I'm going to fix this one time but I honestly find your review comments below to be extremely unproductive.  Changes like that should be done at the importer script level, not by humans.  You've literally wasted 5 minutes of my time for no benefit whatsoever because in practice nobody is going to read these files, and there are a lot more .log files.  In the future, please consider refrain from leaving these review comments.

(In reply to comment #7)
> Comment on attachment 277324 [details]
> Reimports tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=277324&action=review
> 
> I know that the w3c-import.log files are generated by running
> import-w3c-tests. We should fix the text in these files to read well.
> 
> > LayoutTests/imported/w3c/web-platform-tests/html/syntax/parsing/template/additions-to-foster-parenting/w3c-import.log:2
> > +Do NOT modify these tests directly in Webkit.
> 
> Webkit => WebKit
> 
> > LayoutTests/imported/w3c/web-platform-tests/html/syntax/parsing/template/additions-to-foster-parenting/w3c-import.log:7
> > +Then run the Tools/Scripts/import-w3c-tests in Webkit to reimport
> 
> Webkit => WebKit
> 
> Also missing a period at the end of this sentence.
> 
> > LayoutTests/imported/w3c/web-platform-tests/html/syntax/parsing/template/additions-to-foster-parenting/w3c-import.log:9
> > +Do NOT modify or remove this file
> 
> Missing a period at the end of this sentence.
Comment 9 Ryosuke Niwa 2016-04-25 21:22:31 PDT
Committed r200072: <http://trac.webkit.org/changeset/200072>
Comment 10 Chris Dumez 2016-04-26 12:36:47 PDT
(In reply to comment #8)
> I'm going to fix this one time but I honestly find your review comments
> below to be extremely unproductive.  Changes like that should be done at the
> importer script level, not by humans.  You've literally wasted 5 minutes of
> my time for no benefit whatsoever because in practice nobody is going to
> read these files, and there are a lot more .log files.  In the future,
> please consider refrain from leaving these review comments.

A bit harsh :/ I also do these sort of r+ with nit fix comments all the time. This is fairly common. Someone spent the time reviewing this big patch, seems a bit unfair to react this way IMHO.