[Webkit-unassigned] [Bug 169449] run-bindings-tests does not write FAIL lines for a test where IDL generation failed

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=169449

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cdumez at apple.com
            Summary|run-bindings-tests does not |run-bindings-tests does not
                   |always report the names of  |write FAIL lines for a test
                   |failed tests                |where IDL generation failed

--- Comment #1 from Darin Adler <darin at apple.com> ---
It's not true that the test output does not point to a specific failure. While the syntax is messy, the information is here:

Next token should be ,, but > on line 2:     maplike<DOMString>; IDLParser.pm:1622 at /Volumes/Data/src/OpenSource/Source/WebCore/bindings/scripts/IDLParser.pm line 216.
 in /Volumes/Data/src/OpenSource/Source/WebCore/bindings/scripts/test/TestMapLike.idl at /Volumes/Data/src/OpenSource/Source/WebCore/bindings/scripts/IDLParser.pm line 216.

The message above says the failure is in TestMapLike.idl, on line 2.

But of course the way the IDL parser reports this is messy and you found it unnecessarily confusing. We have put very little time and thinking into good error handling for the IDL parser. The code for this is in IDLParser.pm, the assert function and other functions that call this. We can change this around, but when doing so we should keep in mind that the kind of message that most helps someone implementing the IDL parser is not probably the same format useful to someone making changes to IDL files.

As far as the run-bindings-tests script itself is concerned, when generate-bindings.pl fails, run-bindings-tests assumes that the error output from the IDL parser is self explanatory. This is distinct from when the failure is due to successful IDL generation with incorrect output. In that case, the bindings test script definitely needs to write something, since otherwise there is no indication of failure. The PASS/FAIL output comes from the code that compares output to expected output; nothing is written at all if the IDL generation fails entirely.

The run-bindings-tests code in question is the run_tests function in the BindingsTests class in Tools/Scripts/webkitpy/bindings/main.py and currently the printing of FAIL and PASS is done entirely by the detect_changes function. We can add printing for the case where IDL generation fails here if we like. That's the line where it calls "passed = False" after calling generate_from_idl, but does not print anything out. Printing out a line that looks like the FAIL line for when comparison fails would be OK with me, although I would probably not make that change because I don't think writing even more is the best way out of this problem.

I see some other problems in the run_tests function. One problem is that if generate_from_idl fails we still call detect_changes. That does not seem right.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170312/39de20e2/attachment.html>