nrwt: call executive.run_command instead of subprocess.popen to generate json results
note that we need to make sure that this works for chromium checkouts (since this code is used for chromium tests that aren't webkit checkouts); possibly that's why it didn't work before.
I think the problem before was that the SCM code didn't work correctly under cygwin SVN. IIRC that has been fixed.
We wouldn't care about cygwin svn for chromium, I don't think, since we don't run cygwin svn on the bots (just win32 svn). Users might be running cygwin svn, but they presumably wouldn't be generating json results. OTOH, apple win does use cygwin svn, so we should presumably make sure it works there.
resetting the owner in case someone else wants to take a look, as these bugs aren't on my immediate to-do list.
Created attachment 156625 [details] Using detect_scm_system The current code checks for the .svn subdirectory in the LayoutTests directory. This doesn't work out well if using Subversion 1.7 or newer. OTOH, using detect_scm_system to detect the correct SCM also makes this work when Git is being used. Is this approach possible or are there some other caveats?
(In reply to comment #5) > Is this approach possible or are there some other caveats? One I just discovered is the resulting test-webkitpy failures. [917/1577] webkitpy.layout_tests.run_webkit_tests_integrationtest.MainTest.test_repeat_each_iterations_num_tests erred: Traceback (most recent call last): ... File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py", line 388, in _get_svn_revision scm = detect_scm_system(in_directory, filesystem=self._filesystem) File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/common/checkout/scm/detection.py", line 81, in detect_scm_system return SCMDetector(filesystem, Executive()).detect_scm_system(path, patch_directories) File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/common/checkout/scm/detection.py", line 69, in detect_scm_system if SVN.in_working_directory(absolute_path): File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/common/checkout/scm/svn.py", line 93, in in_working_directory exit_code = Executive().run_command(svn_info_args, cwd=path, return_exit_code=True) File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/common/system/executive.py", line 402, in run_command close_fds=self._should_close_fds()) File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/common/system/executive.py", line 458, in popen return subprocess.Popen(*args, **kwargs) File "/usr/lib/python2.7/subprocess.py", line 679, in __init__ errread, errwrite) File "/usr/lib/python2.7/subprocess.py", line 1249, in _execute_child raise child_exception OSError: [Errno 2] No such file or directory: '/test.checkout/LayoutTests' common.checkout.scm.svn runs commands on the real filesystem and not the mock filesystem used in settings.
Created attachment 161689 [details] Patch
Comment on attachment 161689 [details] Patch LGTM.
Comment on attachment 161689 [details] Patch Clearing flags on attachment: 161689 Committed r127302: <http://trac.webkit.org/changeset/127302>
All reviewed patches have been landed. Closing bug.
This caused me to get the following error whenever I run the tests. I have a WebKit checkout embedded in a Chromium checkout. Don't know if that matters. Also, I'm on Ubuntu Precise. It's failing to run because /work/chromium/src/build does not exist. Not sure where that path is coming from. ScriptError raised: Failed to run "[u'git', u'log', u'-25', u'/work/chromium/src/build']" exit_code: 128 File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 110, in run unexpected_result_count = manager.run(args) File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 450, in run self._upload_json_files(summarized_results, result_summary, individual_test_timings) File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 570, in _upload_json_files self._options.master_name) File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py", line 76, in __init__ self.generate_json_output() File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py", line 244, in generate_json_output json_object = self.get_json() File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py", line 284, in get_json self._insert_generic_metadata(results_for_builder) File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py", line 529, in _insert_generic_metadata self._get_svn_revision(path), File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py", line 391, in _get_svn_revision return scm.svn_revision(in_directory) File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/scm/git.py", line 250, in svn_revision git_log = self.run([self.executable_name, 'log', '-25', path]) File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/scm/scm.py", line 76, in run decode_output=decode_output) File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 423, in run_command (error_handler or self.default_error_handler)(script_error) File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 340, in default_error_handler raise error Failed to execute Tools/Scripts/new-run-webkit-tests at /work/chromium/src/third_party/WebKit/Tools/Scripts/run-webkit-tests line 124.
That path is coming from chromium.py's implementation of repository_paths() ... how can you have a chromium checkout that doesn't have src/build ? (I just tripped over this myself, but this was from running the chromium port in a webkit checkout that had a hand-copied set of binaries and not a full chromium checkout).
(In reply to comment #12) > That path is coming from chromium.py's implementation of repository_paths() ... how can you have a chromium checkout that doesn't have src/build ? > > (I just tripped over this myself, but this was from running the chromium port in a webkit checkout that had a hand-copied set of binaries and not a full chromium checkout). I have the same problem. src/build is exists, but git log fails because src/build is outside repository when you put unmanaged WebKit repository in chromium's third_party directory. I have a patch to fix this. Will upload soon.
I think the patch is OK but the error occurs as expected in such conditions (copying the WebKit checkout into Chromium checkout and similar). I'm not sure what a proper fix would be, though. Seeing that using different checkouts together is common, that should definitely be taken into account.
Could we roll out until we have a proper fix?
(In reply to comment #15) > Could we roll out until we have a proper fix? I posted a proposal fix. https://bugs.webkit.org/show_bug.cgi?id=95662