Closed
Bug 493792
Opened 16 years ago
Closed 15 years ago
refactor runreftests.py test harness
Categories
(Testing :: Reftest, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jmaher, Assigned: jmaher)
References
Details
Attachments
(1 file, 6 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
The reftest suite depends a lot on python libraries and commands which do not exist on Windows Mobile. In my initial hacking to get this running, I have modified runreftest.py and automation.py extensively to remove dependencies on popen, tmpdir, and the '/'. When I get reftests running end to end, I will post example versions of runreftest.py and automation.py here so we can figure out how best to address this problem.
Assignee | ||
Comment 1•16 years ago
|
||
This is the set of changes made to automation.py in order to run reftest on windows mobile with pythonce and osce. In addition, we will also need runrt.py which is a slightly modified version of runreftests.py. this is submitted as a WIP and for education purposes only.
Assignee | ||
Comment 2•16 years ago
|
||
This is the set of changes made to runreftest.py in order to run reftest on windows mobile with pythonce and osce. In addition, we will also need automation.py this is submitted as a WIP and for education purposes only.
Assignee | ||
Comment 3•15 years ago
|
||
run with this: python \tests\reftest\runreftests.py --appname=\tests\fennec\fennec.exe --manifest=\tests\reftest\tests\layout\reftests\reftest.list
Attachment #382750 -
Attachment is obsolete: true
Comment 5•15 years ago
|
||
Comment on attachment 399576 [details] updated automation.py > status = proc.wait() > y > if status != 0: This y is a problem for the parser. I deleted it, but wanted to point it out.
Assignee | ||
Comment 6•15 years ago
|
||
patch to refactor runreftests.py so we can successfully run reftests via a subclass.
Assignee: nobody → jmaher
Attachment #399575 -
Attachment is obsolete: true
Attachment #399576 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #426722 -
Flags: review?(ted.mielczarek)
Assignee | ||
Updated•15 years ago
|
Summary: need to rewrite python harness to get reftests running on windows mobile → refactor runreftests.py test harness
Comment 7•15 years ago
|
||
Comment on attachment 426722 [details] [diff] [review] refactor runreftests.py >diff --git a/layout/tools/reftest/runreftest.py b/layout/tools/reftest/runreftest.py >--- a/layout/tools/reftest/runreftest.py >+++ b/layout/tools/reftest/runreftest.py >@@ -39,33 +39,37 @@ > > """ > Runs the reftest test harness. > """ > > import sys, shutil, os, os.path > SCRIPT_DIRECTORY = os.path.abspath(os.path.realpath(os.path.dirname(sys.argv[0]))) > sys.path.append(SCRIPT_DIRECTORY) >+os.chdir(SCRIPT_DIRECTORY) Bleh, I wish we didn't need this at all. Do we actually still rely on it? >+ def getManifestPath(self, path): >+ return self.getFullPath(path) >+ I assume you're doing something useful with this in your subclass? I think you should add a comment or a docstring here describing what this does, since this specific implementation doesn't do anything special. > def runTests(self, manifest, options): > debuggerInfo = getDebuggerInfo(self.oldcwd, options.debugger, options.debuggerArgs, > options.debuggerInteractive); > > profileDir = None > try: > profileDir = mkdtemp() > self.createReftestProfile(options, profileDir) > self.copyExtraFilesToProfile(options, profileDir) >+ browserEnv = self.buildBrowserEnv(options, profileDir) > >+ self.registerExtension(browserEnv, options, profileDir) > # Remove the leak detection file so it can't "leak" to the tests run. > # The file is not there if leak logging was not enabled in the application build. >- if os.path.exists(leakLogFile): >- os.remove(leakLogFile) >+ if os.path.exists(self.leakLogFile): >+ os.remove(self.leakLogFile) Just move this "rm the leak log" bit up into the registerExtension() method (or at least factor it into another method, like removeLeakLog() that gets called from registerExtension().) r=me with those changes.
Attachment #426722 -
Flags: review?(ted.mielczarek) → review+
Comment 8•15 years ago
|
||
(In reply to comment #7) > Bleh, I wish we didn't need this at all. Do we actually still rely on it? Notwithstanding the make target, I think people still find it useful for running the script by hand with arguments other than those provided by default by that target.
Comment 9•15 years ago
|
||
Still, the script knows where it is, it should be able to locate all the necessary modules and files without having to chdir there.
Comment 11•15 years ago
|
||
Don't think so, I'm just complaining about the use of chdir. I'm pretty sure we can remove that without doing the work that that other bug requires.
Assignee | ||
Comment 12•15 years ago
|
||
updated patch with added comment and moved the leaklog cleanup to registration function. For the os.chdir I filed bug 549992 to track that issue as it requires a bit more due diligence before ripping it out completely.
Attachment #426722 -
Attachment is obsolete: true
Attachment #430138 -
Flags: review+
Comment 13•15 years ago
|
||
Checked in as a6a6824e2a0e --> FIXED
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 14•15 years ago
|
||
Backed out -->REOPEN. Needs to be tested in a make-package scenario.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 15•15 years ago
|
||
fixed bug on unittests machines where os.chdir was getting the wrong directory for firefox. I removed the os.chdir as requested in the original review and testing in objdir, packaged build, and try server.
Attachment #430138 -
Attachment is obsolete: true
Comment 16•15 years ago
|
||
Checked in as: 1e59db12e921
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•