[webkit-reviews] review requested: [Bug 46453] [NRWT] Put the http and websocket tests to end of the test list : [Attachment 69191] proposed_patch_v2

[bugzilla-daemon at webkit.org]

Gabor Rapcsanyi <rgabor at inf.u-szeged.hu> has asked  for review:
Bug 46453: [NRWT] Put the http and websocket tests to end of the test list
https://bugs.webkit.org/show_bug.cgi?id=46453

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

------- Additional Comments from Gabor Rapcsanyi <rgabor at inf.u-szeged.hu>
Look at the actual code from run_webkit_tests.TestRunner:

> WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:514
>	 if (self._options.experimental_fully_parallel or
>	     self._is_single_threaded()):
>	     filename_queue = Queue.Queue()
>	     for test_file in test_files:
>		 filename_queue.put(
>		     ('.', [self._get_test_info_for_file(test_file)]))
>	     return filename_queue

This will put all tests to separate groups to the queue one by one, I think it
is a better way if we put all tests to the same group, because the separate
groups cause overhead in dump_render_tree_thread.

>
>	 tests_by_dir = {}
>	 for test_file in test_files:
>	     directory = self._get_dir_for_test_file(test_file)
>	     tests_by_dir.setdefault(directory, [])
>	     tests_by_dir[directory].append(
>		 self._get_test_info_for_file(test_file))
>
>	 # Sort by the number of tests in the dir so that the ones with the
>	 # most tests get run first in order to maximize parallelization.
>	 # Number of tests is a good enough, but not perfect, approximation
>	 # of how long that set of tests will take to run. We can't just use
>	 # a PriorityQueue until we move # to Python 2.6.
>	 test_lists = []
>	 http_tests = None
>	 for directory in tests_by_dir:
>	     test_list = tests_by_dir[directory]
>	     # Keep the tests in alphabetical order.
>	     # TODO: Remove once tests are fixed so they can be run in any
>	     # order.
>	     test_list.reverse()
>	     test_list_tuple = (directory, test_list)
>	     if directory == 'LayoutTests' + os.sep + 'http':
>		 http_tests = test_list_tuple

This section is unreachable because directory will never contain the
'LayoutTests' word.
The comment says it will put the http tests first but it won't do anything
because the http_tests will always be empty.

>	     else:
>		 test_lists.append(test_list_tuple)
>	 test_lists.sort(lambda a, b: cmp(len(b[1]), len(a[1])))
>
>	 # Put the http tests first. There are only a couple hundred of them,
>	 # but each http test takes a very long time to run, so sorting by the
>	 # number of tests doesn't accurately capture how long they take to
run.
>	 if http_tests:
>	     test_lists.insert(0, http_tests)


In this patch I put the http and websocket tests to the end of the list if we
use NRWT single threaded like in ORWT.
In multi-threaded mode it will put these tests first in the queue.