[webkit-reviews] review granted: [Bug 98422] [TestResultServer] Add support for non-Chromium TestExpectations files : [Attachment 168403] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Zan Dobersek
<zandobersek at gmail.com>'s request for review:
Bug 98422: [TestResultServer] Add support for non-Chromium TestExpectations
files
https://bugs.webkit.org/show_bug.cgi?id=98422

Attachment 168403: Patch
https://bugs.webkit.org/attachment.cgi?id=168403&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=168403&action=review


Thanks for fixing this and being so responsive to the code review comments!

> Tools/TestResultServer/static-dashboards/flakiness_dashboard.js:52
> +	       'ANDROID': { fallbackPlatforms: ['CHROMIUM'],
expectationsDirectory: 'chromium-android' }

Shouldn't this be ['CHROMIUM', 'CHROMIUM_ANDROID'] ?

> Tools/TestResultServer/static-dashboards/flakiness_dashboard_unittests.js:773

> +    var trie = new TestTrie(builders, resultsByBuilder);
> +    deepEqual(trie._trie, expectedTrie);

Mind also adding a couple test of TestTrie.forEach (one that takes a startpath
and one that doesn't)?