[Webkit-unassigned] [Bug 162458] EWS should run JavaScriptCore tests

[bugzilla-daemon at webkit.org]

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

--- Comment #36 from Aakash Jain <aakash_jain at apple.com> ---
Comment on attachment 301069
  --> https://bugs.webkit.org/attachment.cgi?id=301069
Patch

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

> Tools/Scripts/webkitpy/common/net/jsctestresults.py:32
> +        intersection_api_tests_passed = first._all_api_tests_passed or second._all_api_tests_passed

please verify whether 'or' or 'and' should be used for intersection.

> Tools/Scripts/webkitpy/common/net/jsctestresults.py:44
> +            return None

might be a good idea to log a warning as well.

> Tools/Scripts/webkitpy/common/net/jsctestresults_unittest.py:31
> +        empty_string = ""

might consider renaming empty_string to empty_json to make it more consistent.

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:45
> +        self.failure_status_id = 42

any specific reason to choose 42? would it make more sense to keep failure id as -1?

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:52
> +        else:

'else' is unnecessary since if returns a value.

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:99
> +        self.assertEqual(return_value, True)

should use assertTrue(return_value)

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:119
> +        self.assertEqual(return_value, True)

same, should use assertTrue(return_value)

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:129
> +        self.assertEqual(return_value, True)

same, should use assertTrue(return_value)

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:139
> +        self.assertEqual(return_value, True)

same, should use assertTrue(return_value)

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:159
> +        self.assertEqual(return_value, False)

same, should use assertFalse(return_value)

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:169
> +        self.assertEqual(return_value, False)

same, should use assertFalse(return_value)

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:179
> +        self.assertEqual(return_value, False)

same, should use assertFalse(return_value)

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:188
> +        self.assertEqual(return_value, True)

same, should use assertTrue(return_value)

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:198
> +        self.assertEqual(return_value, True)

same, should use assertTrue(return_value)

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:209
> +        self.assertEqual(task.test_run_count(), 3)

don't we need to assert the return_value here?

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:218
> +        self.assertEqual(return_value, True)

same, should use assertTrue(return_value)

> Tools/Scripts/webkitpy/tool/steps/checkpatchrelevance.py:40
> +    jsc_paths = [

can consider renaming it to jsc_relevant_paths

> Tools/Scripts/webkitpy/tool/steps/checkpatchrelevance.py:47
> +    def _does_contain_change_in_paths(self, files, patterns):

_does_contain_change_in_paths is  misleading name (we are not searching for change in path here)

> Tools/Scripts/webkitpy/tool/steps/checkpatchrelevance.py:48
> +        for item in files:

renaming item to file would make it more readable.

-- 
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/20170210/a55368f1/attachment.html>