Open Bug 434365 Opened 16 years ago Updated 2 years ago

Add a commandline option to run Mochitest in a separate X server on Linux

Categories

(Testing :: Mochitest, defect)

Product:
Testing
Component:
Mochitest
Platform:
x86
Linux
Type:
defect

Tracking

(Not tracked)

People

(Reporter: Waldo, Unassigned)

References

(
URL
)

Details

Attachments

(2 files, 1 obsolete file)

errors when running mochitest under xvfb
5.12 KB, text/plain
Details
(rev 2) updated after bug 428009 landing
11.90 KB, patch
ted
: review-
bhearsum
: review+
Details | Diff | Splinter Review 
Running Mochitests takes over your machine due to the requirement that the application window have focus.  If you don't have either free time or multiple machines, it basically means you waste 45 minutes of your life every time you run them.  I don't know details, but it seems possible that with a dash of Xnest or something it should be possible to run tests without taking over a machine.  This bug exists as a place for such effort to live.

This URL has some initial work on running inside a separate X display:

http://groups.google.com/group/mozilla.dev.platform/browse_frm/thread/0e8b8dbf2a0d6fcd

It's supposed to be noticeably faster than running in the main X display, but you have to do a bunch of stuff manually to make it work, and it requires root access.  Nevertheless, it might be a good starting point toward getting something better working.
It can actually be a lot simpler than what the post describes if you just
1) create a separate user account
2) log in as that user on the console (ctrl-alt-F1)
3) startx -- :1
4) xhost +localhost    (from within the new display)
this gives you a second DISPLAY to run mochitest in.  Make sure the mouse is in a corner of the screen to avoid focus problems.  I don't know if it matters, but I also disabled the screensaver for this user.  You can switch between this display and the normal display via ctrl-alt-Fx, (x=7 => normal, x=8 => secondary for me)

To run the test, just do
$ DISPLAY=localhost:1.0
$ export DISPLAY
[run mochitests]

You're running on a real display, so you're not going to get the extra 10%, but you can also actually see it running and perhaps diagnose problems if needed.  You can also do this without a separate account, but I found that linux would occasionally get confused about which display owned the console (access to soundcard).
(In reply to comment #0)
> I don't know details, but it seems possible that with a dash of
> Xnest or something it should be possible to run tests without taking over a
> machine.

AFAICT, most popular modern Linux distros only allow console users to start X. That is the major obstacle to seamless integration of this thing. As a result, "runtest.py" must be run from a text console (Ctrl-Alt-Fn[1-6]), or some root tweaks are required.

The former is not an option personally for me . "ati" driver is broken and switches backlight off for text consoles ATM in Debian/unstable. There maybe other weird reasons like that for different folks, so I've come up with a least insecure workaround: start X as root and give away Xauthority file.

Other than that, the sequence looks quite doable:
1. Guess a free X socket
2. Set DISPLAY and XAUTHORITY
3. startx/Xnest
4. Begin actual tests

The only real difference between startx and Xnest is whether the new server runs on its own virtual console, or inside the current X session.
There's a modern version of Xnest called Xephyr, which can be run by any user with no special privileges.  There's no way (that I can find) to get it to not paint a display at all, but you can focus a window inside the Xephyr window, then minimize the Xephyr window, and the inner window doesn't notice (tested with xev).
(In reply to comment #3)
> There's a modern version of Xnest called Xephyr, which can be run by any user
> with no special privileges.

This works! And Xephyr supports xauth. The new sequence follows:
1. Check that Xephyr is present in $PATH, fall back to same-screen mode on failure.
2. Check $MOZ_DISPLAY, set it to ":1", if zero.
3. Check $MOZ_XAUTH, set it to "~/.Xauth-mochitest", if zero.
4. Check that $MOZ_XAUTH is a file. If not present, create with shell command:
  xauth -f $MOZ_XAUTH add $(hostname)$MOZ_DISPLAY . $(mcookie)
5. Attempt to launch Xephyr with params:
  Xephyr $MOZ_DIPLAY -auth $MOZ_XAUTH -screen 1024x768
   fall back to same-screen mode on failure.
6. Set $DISPLAY to $MOZ_DISPLAY.
7. Start the tests.

I'll try to wrap this in a patch.
(In reply to comment #3)
> There's no way (that I can find) to get it to not
> paint a display at all, but you can focus a window inside the Xephyr window,
> then minimize the Xephyr window, and the inner window doesn't notice (tested
> with xev).

Perhaps Xvfb would avoid the need for this:

http://en.wikipedia.org/wiki/Xvfb
http://www.x.org/archive/X11R6.8.2/doc/Xvfb.1.html
(In reply to comment #5)
> Perhaps Xvfb would avoid the need for this:

Yes!

$ time nice xvfb-run --auto-servernum python runtests.py --log-file=~/mochitest.log --file-level=DEBUG --autorun --close-when-done

appears to Just Work.  (I'll follow up when it's done.)
Ran to completion in just under an hour - this may be slower than the ideal case since my laptop was unplugged for part of it.

There are a bunch of failures, though.  I don't know if these are "normal", reflect issues with my environment, or are genuine issues with running the tests under xvfb.  I think I've seen the widgets/ timeouts when running the tests in the usual way.
(In reply to comment #6)
> $ time nice xvfb-run --auto-servernum python runtests.py
> --log-file=~/mochitest.log --file-level=DEBUG --autorun --close-when-done
> 
> appears to Just Work.  (I'll follow up when it's done.)

Yeah! GREAT! Even ' --auto-servernum' seems to be redundant! And it is at least as fast as with "dummy" video driver.

Thanks, Karl. Thanks, Zack.
So can we distill that down to maybe

$ python runtests.py --vfb --log-file=mochitest.log 

where --vfb, in addition to going out and invoking xvfb-run, implies --autorun --close-when-done and a sensible --file-level?

Say, why is there both runtests.py and runtests.pl?
(In reply to comment #9)
> So can we distill that down to maybe
> 
> $ python runtests.py --vfb --log-file=mochitest.log 
> 
> where --vfb, in addition to going out and invoking xvfb-run, implies --autorun
> --close-when-done and a sensible --file-level?

'xvfb-run' fully implements the sequence from comment 4, saving us all the work. We will either need to fork xvfb-run from python or re-implement the sequence. Neither seems to be worth the efforts. Your previous command line is sufficiently concise.

> Say, why is there both runtests.py and runtests.pl?

http://developer.mozilla.org/en/docs/Mochitest#Running_the_whole_test_suite says the latter is obsolete ;)
Forking xvfb-run from python is exactly what I had in mind.  I think there is sufficient value in doing that: first, it's much easier to document and remember; second, if we ever discover a way to do something similar on OSX or Windows, we can put that under the same option.
Because sayrer and I sinned greatly in our youth.  I've been trying for the last several months to get tinderboxen to switch to runtests.py so runtests.pl can die a painful death (stab stab stab), but no dice so far at getting tinderbox maintenance people to make the switch, even given new features like leak detection (or, soon, the forthcoming SSL support of bug 428009).  I'm not spending time writing patches which touch the Perl version, and I do little more than skim patches to it when I get review requests.  I wasted far too much time relearning Perl every time I wrote a patch to it, whereas with Python I can actually read and write the code with little effort and no mental pain.  Ignore the Perl version; its days are numbered.

I'd prefer a more readable name (and one which would work cross-platform, since there's got to be something like this for Windows and OS X) for the option, but I'm not sure what that name is.  I'd also rather it not implied any other options; I can think of good reasons why you might not always want them.

The idea of making the option a *negative* option appeals to me, since otherwise people may have a hard time learning about it, but one step at a time here, I suppose.  :-)
(In reply to comment #12)
> I'd prefer a more readable name (and one which would work cross-platform, since
> there's got to be something like this for Windows and OS X) for the option, but
> I'm not sure what that name is.  I'd also rather it not implied any other
> options; I can think of good reasons why you might not always want them.
> 
> The idea of making the option a *negative* option appeals to me, since
> otherwise people may have a hard time learning about it, but one step at a time
> here, I suppose.  :-)

I still think runtest.py option will be redundant:
It won't work without --autostart and --close-when-done with Xvbf, because there is no way to interact this the program after it starts.

Since graphics is integrated into Windows kernel, we are unlikely to see anything similar to Xvfb on that platform with current versions. Cannot say anything about OS X. Virtualized solutions are still possible, but managing a virtual machine is a completely different task.

Finally, Xvfb doesn't seem to be installed by default on Debian and Ubuntu. I suspect the case is the same for most other distros. So the likely way to advertise this option is to fail with a message like:

xfvb-run not found in PATH. Please install xfvb package or run with --disable-vfb option.

I also don't like forking in this case, because it may lead to unattached processes. The counter-proposal to address verbosity is to put the command line into a simple shell script, which can be placed into the same dir as runtest.py:

./runtest-xvfb.sh

Anyway, thanks again for this solution. I've advertised the command from comment 9 in Mochitest docs on MDC [0]. Please update with whatever choice will be made here.

0. http://developer.mozilla.org/en/docs/Mochitest#Running_the_whole_test_suite
Hm, I wasn't familiar enough with Xvfb to know it didn't pop up a window while running and didn't accept events.  Given those things, how about a  --background argument that implies autorun/close-when-done as suggested?  We can print a nice error about "Xvfb not in PATH" if the argument's supplied but Xvfb isn't in the PATH.  It sort of worries me we have no way to watch a run in this mode; there are a ton of ways you could get a failure that's simply invisible in this mode of operation.  I guess we can deal in a followup or something if we find it's a big problem.

It's too easy to blow off Windows for its architectural choices; I have a very hard time believing it's not possible there.  Ditto with respect to OS X and Quartz, which I hear is fairly decent as these things go.  The problem might be hard, but we experience enough pain from it I'll bet against our never being able to solve it.
Having this would make an implementation of bug 417516 much nicer, because you could just run "make mochitest" and get back a result without it tying up your machine in the process.

(In reply to comment #14)

> It's too easy to blow off Windows for its architectural choices; I have a very
> hard time believing it's not possible there.

It might be, Windows Terminal Services seems to be the magic term But I'm not at all a Windows guy, so my understanding ends there.

BTW, AFAIK Xvfb is already used on the Linux test boxes. Maybe check with QA to see about integrating that with the work here?

(In reply to comment #14)
> It's too easy to blow off Windows for its architectural choices; I have a very
> hard time believing it's not possible there.
You can probably create a desktop (or possibly a window station, although that looks like overkill). The documentation for CreateProcess is a little confusing as it claims that by default the new process inherits the desktop from its parent process although the desktop is apparently a per-thread setting (a window station is per process).
I am still pessimistic about parallel run on Windows without virtualization.

The main problem is the focus. We have one focused window per X server, so 1 kernel -- 2 X servers -- 2 focused windows on Linux. But IIUC there can only be one focused window per Windows kernel. So if we want 2 focused windows, we need 2 Windows kernel, and that' virtualization.
Here's a patch which adds an --offscreen switch to the mochitest runner.  When automation.UNIXISH is true, this sets up an Xvfb instance with its own window manager and runs the test application in there.  On other systems it just errors out -- as discussed upthread, there may well be a way to do it, but I'm not the man to implement it.  

You don't need xvfb-run, just Xvfb itself, xauth, and a window manager.  There is a hardcoded list of window managers that work; presently the only item on the list is metacity.  Please feel free to augment it -- but make sure it works first.  twm does NOT work -- the browser hangs on startup.  Testing with your regular desktop environment is not good enough; you have to try it with the somewhat peculiar environment this sets up, to be sure.

The list of environment variables to be unset in child processes (prepare_isolation in automation.py) needs work by people familiar with non-Gnome desktop environments -- see the comments there.

It would be good to arrange for the mochitest server process to be killed if xvfb or the window manager won't start, but that would require more restructuring of the code than I felt like doing in this patch.
Assignee: nobody → zweinberg
Status: NEW → ASSIGNED
Attachment #335143 - Flags: review?(rcampbell)
before I get neck deep in this review, is there any reason the user can't start Xvfb on the command-line, run metacity and then run their mochitests using DISPLAY=:1 runtests.py? We do this on the unittest machines and it works very well and doesn't require platform-specific modifications to the test harness.
I was doing just that before I wrote this patch.  The trouble is that you have to set everything up just right, or the tests fail.  For example, I had to discover by trial and error that xvfb-run's default 800x600 framebuffer is not big enough, and that you have to prevent both the metacity instance and the browser itself from communicating with any GNOME desktop daemons running in the user's regular session.

So the patch's chief value in my mind is wrapping all of that up neatly: just use --offscreen and you'll get a configuration that's known to work.
I'm in favor of this approach (although I haven't looked at this specific patch). I have a partial implementation of this for Windows in bug 443030 as well.

(In reply to comment #22)
> I'm in favor of this approach (although I haven't looked at this specific
> patch). I have a partial implementation of this for Windows in bug 443030 as
> well.

which approach, ted? Bundling the necessary windowing code into the harness? I suppose it makes it easier for people, although some better instructions on how to run this stuff might also do the trick without bloating the code.

Zack: There are a couple of tricks, for sure. We usually use 1024x768x24 for our window geometry. Reftests require the 24 bits depth.
Sorry, the approach suggested by this bug and presumably Zack's patch. I'd like it to be as easy as possible to run mochitests in a non-invasive manner.

I don't think of it as bloat; I would much rather have automation than instructions.  My ultimate goal here (and I think ted agrees) is that a casual contributor could type "make check" at top level of a build tree, minimize that window and do something else for a couple hours, and then come back and find *all* the tests run and a nice little summary showing in the terminal.  They shouldn't have to know any "tricks" and they shouldn't have to set things up in advance.

I see that this does add a lot of Unix-specific code in runtests.py.  Would you be happier with that if it were over in automation.py?  A new file altogether?

The patch currently specifies 1024x1024x8, mostly because xvfb-run used that bit depth; I shall try 1024x768x24; it occurs to me that Cairo is probably optimized for 24-bit color a lot more than 8-bit, and so it might even be faster.
minor revisions so it applies on top of the code for bug 428009.  (which also demonstrated how to flesh out one of the windows stubs, thanks.)
Attachment #335143 - Attachment is obsolete: true
Attachment #337065 - Flags: review?(rcampbell)
Attachment #335143 - Flags: review?(rcampbell)
Comment on attachment 337065 [details] [diff] [review]
(rev 2) updated after bug 428009 landing

as long as this doesn't break the current, non-offscreen implementation, this looks like it should work. Requesting additional review from ted who knows automation.py.
Attachment #337065 - Flags: review?(ted.mielczarek)
Attachment #337065 - Flags: review?(rcampbell)
Attachment #337065 - Flags: review+
Attachment #337065 - Flags: review?(bhearsum)
Comment on attachment 337065 [details] [diff] [review]
(rev 2) updated after bug 428009 landing

asking for more review help here too since Ben's going to be on the hook if the unittest machines start breaking.
Attachment #337065 - Flags: review+ → review-
Comment on attachment 337065 [details] [diff] [review]
(rev 2) updated after bug 428009 landing

on second thought, I'm recanting this review. I don't trust it.

Using a for loop to find an unused socket in /tmp/.X11-unix seems error-prone and inelegant.

Also, are we sure all linuxes use /tmp/.X11-unix for their socket file?

the xauth addition to xvfb seems over-engineered and unnecessary. Are you sure this works on all linuxes? What about Solaris?

I don't like making changes to the way the mochitest server starts up.

in your comments, ...such as good old twm) and some don't work with no
+# configuration.

should read "some do not work without configuration".
(In reply to comment #29)
> (From update of attachment 337065 [details] [diff] [review])
> 
> Using a for loop to find an unused socket in /tmp/.X11-unix seems error-prone
> and inelegant.

As there is no way to force X (library or server) to use a specified socket pathname, there is no alternative.  This is what xvfb-run does.

> Also, are we sure all linuxes use /tmp/.X11-unix for their socket file?

All X implementations for Unix ultimately derived from the original MIT implementation (which is all of them these days), yes.  It's been wired into libX11 since, um, X11R5 is as far back as I go.

> the xauth addition to xvfb seems over-engineered and unnecessary. Are you sure
> this works on all linuxes? What about Solaris?

I refuse to have anything to do with Solaris anymore, but that part is a verbatim translation of the logic in xvfb-run, which is distributed by X.org, so it should work everywhere.  The format of the .Xauthority file is supposed to be opaque; I don't know any more elegant way to do it.

> I don't like making changes to the way the mochitest server starts up.

Are you referring to running it under the environment created by setup_isolation?  That is in case xpcshell decides to try to talk to a desktop daemon.  I don't know any reason why it would _need_ to, but if it did, and it got different answers from (say) gconf than the browser process did, I could imagine that breaking the tests.  So it's safer to make sure they're both talking to the same absence of daemons.  [Empirically, a gconf instance does start up inside the xvfb environment.  I wish I could be sure it wasn't reading per-user configuration.]

> in your comments, ...such as good old twm) and some don't work with no
> +# configuration.
> 
> should read "some do not work without configuration".

Will change.
Comment on attachment 337065 [details] [diff] [review]
(rev 2) updated after bug 428009 landing

This looks fine. I'm not an X expert, so I don't entirely understanding starting xauth, Xvnc, and the window manager, but I trust that it is the correct behaviour.

With regard to releng unittest machines, will this more or less guarantee we don't hit focus issues? Obviously, this won't affect anything unless we flip it on.

This is great though, especially for developers.
Attachment #337065 - Flags: review?(bhearsum) → review+
I'll defer to Ben and Ted on this one as they're most-affected by it. I still think that comment should get fixed on checkin.
Attachment #337065 - Flags: review-
(In reply to comment #31)
> 
> With regard to releng unittest machines, will this more or less guarantee we
> don't hit focus issues? Obviously, this won't affect anything unless we flip it
> on.

That's what it's supposed to do.  I always see a small number of mochitest failures no matter what - not always the same tests, and I haven't checked whether they have anything to do with focus, but the upshot is I can't give
you any guarantees.
Comment on attachment 337065 [details] [diff] [review]
(rev 2) updated after bug 428009 landing

+  def stop(self):
...
+        import subprocess

Perhaps you missed the comment here:
http://mxr.mozilla.org/mozilla-central/source/build/pgo/automation.py.in#106

+# Window managers to try when using --offscreen mode in Unix.  The
+# first one of these found in $PATH is used.  Don't commit additions
+# to this list unless you've tried them!  Some make the browser hang
+# forever (such as good old twm) and some don't work with no
+# configuration.
+WINDOW_MANAGERS = ('metacity',)

I don't mind having a whitelist of window managers, but let's do detection in configure.in, since it's good at that. You can stick the detected WMs in a make variable, pass them down to this script via the preprocessor, and error out if it's unset. You should also do checks for xauth/Xvfb in the same way.

Any particular reason you put the actual launching of Xvfb in runtests.py instead of automation.py? Seems like it could be useful in the general sense. (Like if we implement a similar wrapper for running reftest.)

As discussed on IRC, I'd be happier requiring xvfb-run rather than duplicating its logic here. At least on Debian I get that from the Xvfb package, so I wouldn't worry about requiring it.

r- for now, but I'm excited for the concept!
Attachment #337065 - Flags: review?(ted.mielczarek) → review-
(In reply to comment #34)
> +        import subprocess
> 
> Perhaps you missed the comment here:
> http://mxr.mozilla.org/mozilla-central/source/build/pgo/automation.py.in#106

That code was copy-and-pasted from the kill() function immediately above; the changeset introducing that unconditional use of subprocess is

  changeset:   18848:70dd0e9f14c2
  user:        Honza Bambas <honzab@allpeers.com>
  date:        Fri Sep 05 09:35:58 2008 -0400
  summary:     bug 428009 - hook up ssltunnel to mochitest. r=waldo,kaie

I skimmed that bug and there doesn't seem to be any discussion of it.  In the revision, I can switch 'em both to use class Process if you want, but I note that the comment you pointed at says "compatibility with python 2.3 _on non-windows platforms_" and this is windows-specific code.

> 
> +# Window managers to try when using --offscreen mode in Unix.  The
> +# first one of these found in $PATH is used.  Don't commit additions
> +# to this list unless you've tried them!  Some make the browser hang
> +# forever (such as good old twm) and some don't work with no
> +# configuration.
> +WINDOW_MANAGERS = ('metacity',)
> 
> I don't mind having a whitelist of window managers, but let's do detection in
> configure.in, since it's good at that.

I am unenthusiastic about this for two reasons:

 - The configure script is already a wretched hive.
 - It means the PATH setting at the time you run runtest.py is not honored.

but I'll do it if you are sure it's better.

> Any particular reason you put the actual launching of Xvfb in runtests.py
> instead of automation.py? Seems like it could be useful in the general sense.
> (Like if we implement a similar wrapper for running reftest.)

No particular reason, and I would like a similar wrapper for reftest, so ok, I'll move it.

> As discussed on IRC, I'd be happier requiring xvfb-run rather than duplicating
> its logic here. At least on Debian I get that from the Xvfb package, so I
> wouldn't worry about requiring it.

Check.

Note that it'll be a while before I can come back to this, I really need to get some ff3.1 feature work done.
Component: Testing → Mochitest
Product: Core → Testing
QA Contact: testing → mochitest
Version: Trunk → unspecified
Irrespective of the fact that not everything uses it yet (I assume), this seems like it would be a good candidate to port to mozrunner.
Resetting owner to default per Zack's request.
Assignee: zackw → nobody
Status: ASSIGNED → NEW

Just my two cents worth.
I am testing TB under linux for like 10 years.
I have used the following setup to run tests in separate window manager inside Xephyr so that I can monitor the progress of local test while I do other things on the desktop. And actually my linux runs inside VirtualBox (previously inside VMPlayer) under Windows.
The whole setup works just fine. (you DO need memory to run this combination).

This is from within a bash script.

    # July 12, 2019. tryserver uses the following size 1600x1200.
    SIZE=1600x1200

#   -br     sets  the default root window to solid black instead of the standard root weave pattern.
#          This is the default unless -retro or -wr is specified.

#       -ac     disables host-based access control mechanisms.  Enables access by any host, and  permits
#               any  host  to  modify  the  access control list.  Use with extreme caution.  This option
#               exists primarily for running test suites remotely.

#       -noreset
#               prevents a server reset when the last client connection is  closed.   This  overrides  a
#               previous -terminate command line option.


    Xephyr -ac -br -noreset -screen $SIZE :2 &
    # was DISPLAY=localhost:2.0 before Oct 27, 2016
    DISPLAY=:2.0
    waitfor 5
    # oclock &
    xfwm4 &

Then I ran the script to invoke mochitest (prevously |make mozmill| test for TB) within the same script. You may need to export DISPLAY variable depending on how your script is written.

I forgot to mention waitfor is a function in the script to wait for given seconds.

function waitfor
{
    i=$1
    # echo "i is $i"
    # should check for numeric, but the only caller is in this file.
    echo "Sleeping for $i seconds."
    while [ $i -gt 0 ]
    do
	echo -n "$i "
	sleep 1
	i=$(( i - 1 ))
    done
}

The verbose wait is to let developer know the sleep is caused by EXPLICT wait by the script, not the
strange sleep caused by the flaky test framework, etc.

xfmw4 is the lightweight window manager that I use. This works fine. (I use it for my desktop, too.). YMMV.
All runs well so far as far as I can tell.

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: