Closed Bug 551256 Opened 10 years ago Closed 10 years ago

Automate VMWare recording over mochitests

Categories

(Testing :: Mochitest, defect)

x86
Windows 7
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.3a4

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached patch enables the guest to start and stop recording over a mochitest run.
Attachment #431437 - Flags: review?(ted.mielczarek)
This is proving very useful, I'd love to get this checked in.
Assignee: nobody → bent.mozilla
Status: NEW → ASSIGNED
Comment on attachment 431437 [details] [diff] [review]
Patch
[Checkin: Comment 4]

>diff --git a/testing/mochitest/runtests.py.in b/testing/mochitest/runtests.py.in
>--- a/testing/mochitest/runtests.py.in
>+++ b/testing/mochitest/runtests.py.in
>@@ -194,16 +196,21 @@ class MochitestOptions(optparse.OptionPa
>                            "(requires a debug build to be effective)")
>     defaults["fatalAssertions"] = False
> 
>     self.add_option("--extra-profile-file",
>                     action = "append", dest = "extraProfileFiles",
>                     help = "copy specified files/dirs to testing profile")
>     defaults["extraProfileFiles"] = []
> 
>+    self.add_option("--use-vmware-recording",
>+                    action = "store_true", dest = "vmwareRecording",
>+                    help = "enables recording while the application is running")

This could probably stand to be a little more verbose. Indicate that this needs to be run inside the guest OS on whatever version of Workstation for Windows.

>@@ -428,16 +436,45 @@ class Mochitest(object):
>       if options.shuffle:
>         self.urlOpts.append("shuffle=1")
> 
>   def cleanup(self, manifest):
>     """ remove temporary files and profile """
>     os.remove(manifest)
>     shutil.rmtree(self.PROFILE_DIRECTORY)
> 
>+  def startVMWareRecording(self, options):

Please add a docstring comment to all functions, even if it's brief.

>+    assert(self.automation.IS_WIN32)
>+    from ctypes import cdll
>+    self.vmwareHelper = cdll.LoadLibrary(
>+      os.path.join(options.utilityPath, VMWARE_RECORDING_HELPER_BASENAME))
>+    if self.vmwareHelper is None:
>+      self.automation.log.warning("WARNING | runtests.py | Failed to load " +
>+                                  "VMWare recording helper")
>+      return
>+    self.automation.log.info("INFO | runtests.py | Starting VMWare recording.")
>+    try:
>+      self.vmwareHelper.StartRecording()
>+    except Exception, e:
>+      self.automation.log.warning("WARNING | runtests.py | Failed to start " +
>+                                  "VMWare recording: (" + str(e) + ")")

Use % string substitution instead of concatenating strings.

>+      self.vmwareHelper = None
>+
>+  def stopVMWareRecording(self):
>+    assert(self.automation.IS_WIN32)
>+    if self.vmwareHelper is not None:
>+      self.automation.log.info("INFO | runtests.py | Stopping VMWare " +
>+                               "recording.")

You don't actually need the + there, Python will concatenate the strings for you anyway.

>@@ -582,16 +626,25 @@ def main():
>     # default xrePath to the app path if not provided
>     # but only if an app path was explicitly provided
>     if options.app != parser.defaults['app']:
>       options.xrePath = os.path.dirname(options.app)
>     else:
>       # otherwise default to dist/bin
>       options.xrePath = automation.DIST_BIN
> 
>+  if options.vmwareRecording:
>+    if not automation.IS_WIN32:
>+      parser.error("use-vmware-recording is only supported on Windows")
>+    helperPath = os.path.join(options.utilityPath,
>+                              VMWARE_RECORDING_HELPER_BASENAME + ".dll")

Why not just save this path on the object instead of re-joining it later?

>+    if not os.path.exists(helperPath):
>+      parser.error("Path " + helperPath + " does not exist, cannot " +
>+                   "automate VMWare recording")

I'd make this a little clearer, like "%.dll not found in utility path %s, cannot automate VMWare recording" % (VMWARE_RECORDING_HELPER_BASENAME, options.utilityPath)

r=me with those changes.
Attachment #431437 - Flags: review?(ted.mielczarek) → review+
Also, you really need to blog about this once you get all the bits landed.
http://hg.mozilla.org/mozilla-central/rev/a8e7a2ba98ff
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a4
Attached patch mingw fix (obsolete) — Splinter Review
This patch broke mingw implementation by using MSC inline assembly. Attached patch implements GCC version of vmwarerecordinghelper.dll implementation. I don't have setup to test this patch in other way than compiling. I don't expect problems with this patch, but I will attach compiled version and it would be great if someone with proper setup could test it.
Attachment #433341 - Flags: review?(ted.mielczarek)
Attached file mingw build (obsolete) —
We should probably just not bother building this if we're using mingw. I would guess that the sets of people interested in both record+replay and mingw users don't intersect.
Thanks for quick review. I've attached a patch that disables vmwarerecordinghelper on mingw.
Attachment #433341 - Attachment is obsolete: true
Attachment #433342 - Attachment is obsolete: true
Attachment #433341 - Flags: review?(ted.mielczarek)
Attachment #433355 - Flags: review?(ted.mielczarek)
Attachment #433355 - Flags: review?(ted.mielczarek) → review+
Keywords: checkin-needed
Attachment #431437 - Attachment description: Patch → Patch [Checkin: Comment 4]
Flags: in-testsuite-
Version: unspecified → Trunk
Comment on attachment 433355 [details] [diff] [review]
fix v2
[Checkin: Comment 9]


http://hg.mozilla.org/mozilla-central/rev/f882c976f8e1
Attachment #433355 - Attachment description: fix v2 → fix v2 [Checkin: Comment 9]
You need to log in before you can comment on or make changes to this bug.