REGRESSION (r137003): failures in MicroData tests on EFL, GTK

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Zan Dobersek]

r137003 introduced 5 failures in MicroData layout tests on GTK and EFL ports (which are, to my knowledge, the only ports enabling the feature).
http://trac.webkit.org/changeset/137003

fast/dom/MicroData/nameditem-must-be-case-sensitive.html
fast/dom/MicroData/nameditem-must-return-correct-item-properties.html
fast/dom/MicroData/nameditem-returns-propertynodelist.html
fast/dom/MicroData/properties-collection-nameditem-test.html
fast/dom/MicroData/propertynodelist-getvalues-test.html

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=fast%2Fdom%2FMicroData

[Chris Dumez]

Skipped for EFL port in http://trac.webkit.org/changeset/137062 and GTK port in http://trac.webkit.org/changeset/137059.

[Ryosuke Niwa]

Created attachment 178425 [details]
Fixes the bug

[Ryosuke Niwa]

Could someone who have access to EFL or GTK port test this patch? Somehow my DRT doesn’t enable micro data bindings properly even though it’s definitely compiling things.

[Ryosuke Niwa]

I think we were treating PropertyNodeList* as if it were Node* prior to r137003. I don’t even know why it worked.

[Chris Dumez]

(In reply to comment #3)
> Could someone who have access to EFL or GTK port test this patch? Somehow my DRT doesn’t enable micro data bindings properly even though it’s definitely compiling things.

Sure, I'm going to give it a try now.

[Chris Dumez]

Created attachment 178427 [details]
Results with the patch applied

Sadly, the patch does not fix the tests. Please see attachment.

[Ryosuke Niwa]

(In reply to comment #6)
> Created an attachment (id=178427) [details]
> Results with the patch applied
> 
> Sadly, the patch does not fix the tests. Please see attachment.

That's surprising. You rebuilt the webkit before running tests, right? (run build-webkit before running run-webkit-tests).

[Chris Dumez]

(In reply to comment #7)
> (In reply to comment #6)
> > Created an attachment (id=178427) [details] [details]
> > Results with the patch applied
> > 
> > Sadly, the patch does not fix the tests. Please see attachment.
> 
> That's surprising. You rebuilt the webkit before running tests, right? (run build-webkit before running run-webkit-tests).

Yes, I did :)

And I tried again and I clearly see that the modified file is being rebuilt:
Scanning dependencies of target webcore_efl
[  9%] Building CXX object Source/WebCore/CMakeFiles/webcore_efl.dir/bindings/js/JSHTMLCollectionCustom.cpp.o
Linking CXX shared library ../../lib/libwebcore_efl.so

However, those 5 MicroData tests are still failing.

[Chris Dumez]

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > Created an attachment (id=178427) [details] [details] [details]
> > > Results with the patch applied
> > > 
> > > Sadly, the patch does not fix the tests. Please see attachment.
> > 
> > That's surprising. You rebuilt the webkit before running tests, right? (run build-webkit before running run-webkit-tests).
> 
> Yes, I did :)
> 
> And I tried again and I clearly see that the modified file is being rebuilt:
> Scanning dependencies of target webcore_efl
> [  9%] Building CXX object Source/WebCore/CMakeFiles/webcore_efl.dir/bindings/js/JSHTMLCollectionCustom.cpp.o
> Linking CXX shared library ../../lib/libwebcore_efl.so
> 
> However, those 5 MicroData tests are still failing.

I tried running fast/dom/MicroData/nameditem-must-be-case-sensitive.html (one of the tests that is failing) and added a printf in JSHTMLCollection::nameGetter(). It appears JSHTMLCollection::nameGetter() is not called.

[Ryosuke Niwa]

Created attachment 178460 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 178460 [details]
Patch for landing

Clearing flags on attachment: 178460

Committed r137107: <http://trac.webkit.org/changeset/137107>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

Ok, I can confirm that the patch that landed fixes the issue for EFL port, thanks.

I unskipped the tests in http://trac.webkit.org/changeset/137116.