nrwt: move actual test-running code into layout_test_runner.py
Created attachment 155665 [details] Patch
Comment on attachment 155665 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=155665&action=review We've got so much yak! > Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:87 > + """Runs the tests. This line is pretty much useless. > Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:93 > + typed Ctrl^C Why are we indenting by 2 space here? > Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:99 > + of each thread with 'name', 'num_tests', 'total_time' properties > + test_timings is a list of timings for each sharded subdirectory > + of the form [time, directory_name, num_tests] > + individual_test_timings is a list of run times for each test > + in the form {filename:filename, test_run_time:test_run_time} Ditto. > Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:113 > + self._all_results = [] > + self._group_stats = {} > + self._worker_stats = {} > + self._has_http_lock = False > + self._remaining_locked_shards = [] Do we really need to re-initialize these variables again? > Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:142 > + def worker_factory(worker_connection): > + results_directory = self._results_directory > + if self._retrying: > + self._filesystem.maybe_make_directory(self._filesystem.join(self._results_directory, 'retries')) > + results_directory = self._filesystem.join(self._results_directory, 'retries') > + return Worker(worker_connection, results_directory, self._options) It's probably better to make this is a private method given that this is a massive function already.
Comment on attachment 155665 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=155665&action=review >> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:87 >> + """Runs the tests. > > This line is pretty much useless. will delete. >> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:93 >> + typed Ctrl^C > > Why are we indenting by 2 space here? no particular reason; this has been this way forever (probably going back to when we used two-space indents in the chromium repo). will fix. >> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:99 >> + in the form {filename:filename, test_run_time:test_run_time} > > Ditto. will fix. >> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:113 >> + self._remaining_locked_shards = [] > > Do we really need to re-initialize these variables again? yes, because this routine gets called twice. >> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:142 >> + return Worker(worker_connection, results_directory, self._options) > > It's probably better to make this is a private method given that this is a massive function already. will do.
Committed r124281: <http://trac.webkit.org/changeset/124281>