[Webkit-unassigned] [Bug 49122] New: webkitpy/tool/* unittests change cwd and don't clean up properly
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Nov 5 20:33:06 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=49122
Summary: webkitpy/tool/* unittests change cwd and don't clean
up properly
Product: WebKit
Version: 528+ (Nightly build)
Platform: PC
OS/Version: Mac OS X 10.5
Status: NEW
Severity: Normal
Priority: P2
Component: Tools / Tests
AssignedTo: webkit-unassigned at lists.webkit.org
ReportedBy: dpranke at chromium.org
CC: eric at webkit.org, abarth at webkit.org
In the course of merging and preparing to land the patch for bug 48280 , I stumbled into this.
various unit tests under webkitpy/tool/* change the current working directory and don't seem to change back when they're done. For example, download_unittest.test_apply_attachment creates a MockSCM() object, and then invokes a series of steps including steps/cleanworkingdirectory.py (which has the effect of changing to /private/tmp ).
This by itself isn't that bad.
However, the real scm checkout code uses the current working directory to figure out where the checkout root is, in default_scm():
def default_scm():
cwd = os.getcwd()
scm_system = detect_scm_system(cwd)
if not scm_system:
script_directory = os.path.abspath(sys.path[0])
scm_system = detect_scm_system(script_directory)
if scm_system:
log("The current directory (%s) is not a WebKit checkout, using %s" % (cwd, scm_system.checkout_root))
else:
error("FATAL: Failed to determine the SCM system for either %s or %s" % (cwd, script_directory))
return scm_system
The first part fails, so we fall back to os.path.abspath(sys.path[0]).
This by itself also isn't that bad. Normally, in python, sys.path[0] is defined to be the directory of the script that was invoked (i.e., test-webkitpy, so WebKitTools/Scripts) .
Sadly, in the monkeying with paths that we do now in test-webkit that seems to have something to do with making sure that QueueStatusServer is in our path, we call out to the WebKitTools/QueueStatusServer/__init__.py:fix_sys_path() which manipulates sys.path .
Even that's not so bad. Sadly, that code also calls google_appengine.fix_sys_path(). And THAT code, at least in the version I happen to have installed on my machine (1.3.8), *prepends* EXTRA_PATHS to sys.paths, which breaks the assumption that sys.path[0] will be inside the tree.
So, if any test code then attempts to call the real scm.default_scm() code, it will fail. Which happens after the above unittests execute (and we've changed the directory), when we run commands/rebaseline_unittest, when that code constructs a layout_tests/port object, which calls the real scm code when I've merged in the changes in 48280.
A comedy of cascading errors, so to speak.
There are several things that can/should be fixed here.
(1) The AppEngine code is probably wrong.
(2) the tool/commands unittests should not actually be changing directories.
(3) It may be bad to be relying only on sys.path[0] and cwd, since clearly both of those things can be outside the tree. The only other mechanism I can think of which should be safe would be to look at __file__ from inside scm.py . I think there are times when even that doesn't work right, because there was a reason I started using sys.path[0] instead of __file__ , but those reasons may not apply in this usage. I don't remember exactly what they were.
(4) Arguably the rebaseline_unittest command should be trying to get a test port, not a real port, and it should be passing in a MockConfig() object so that we don't use the real scm detection code. But there might need to be other stuff that changes in that test, I haven't really explored this yet.
I think (3) is safe, so I'll upload a patch for that.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list