[Webkit-unassigned] [Bug 60869] Convert json_results_generator.py to output version 4 JSON.

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Alice Boxhall <aboxhall at chromium.org>  2011-05-18 18:40:40 PST ---
(From update of attachment 93603)
View in context: https://bugs.webkit.org/attachment.cgi?id=93603&action=review

>> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:88
>> +            if not isinstance(data, dict) or len(data) == 0 or "results" in data:
> 
> Do we ever hit cases in practice where data is not a dict or where len(data) == 0?
> 
> Also, !len(data) is preferred to checking if it's equal to zero in WebKit code.

By the time this code is called, it should always have a results dict for each test, but there are stages during building where it's an empty dict. I'm not sure where to draw the line with being cautious about what format the data will take.

Changed to not len(data).

>> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:96
>> +def convert_flat_path_to_trie(path, value, trie):
> 
> nit: Maybe just call this add_path_to_trie? It's not really converting anything to a trie.

Done.

>> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:108
>> +    directory, _, rest = path.partition("/")
> 
> Would be good to name the middle variable even if it's unused. Just to know what's going on.

Done.

>> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:314
>> +        all_failing_tests.update(convert_trie_to_flat_paths(tests))
> 
> I don't understand what this is doing. Sorry, I tried to figure it out.

Hm. That's because it has three completely unnecessary lines. Removed.

>> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:573
>> +        if len(thisTest) == 0:
> 
> if !len(thisTest):

Done.

>> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:606
>> +                    continue
> 
> Why do you need a try/except here?

Cargo cult copied from the old code (in json_layout_results_generator.py). I guess sometimes there is no tests data? A check would be a better way to handle that, though.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.