# run mozharness in XVfb, if necessary; this is an array to maintain the quoting in the -s argument if $NEED_XVFB; then # Some mozharness scripts set DISPLAY=:2 Xvfb :2 -screen 0 1024x768x24 & export DISPLAY=:2 xvfb_pid=$! # Only error code 255 matters, because it signifies that no # display could be opened. As long as we can open the display # tests should work. sleep 2 # we need to sleep so that Xvfb has time to startup xvinfo || if [ $? == 255 ]; then exit 255; fi fi The "sleep" is going to be flaky. We should, instead, poll for Xvfb startup until some suitably long time (a minute, say).

Comment on attachment 8607616 [details] [diff] [review] xvfb.retry.diff Review of attachment 8607616 [details] [diff] [review]: ----------------------------------------------------------------- Perhaps xvinfo never exits 0, but just in case, this probably deserves an update.. ::: testing/docker/desktop-build/bin/build.sh @@ +85,5 @@ > + retry_count=0 > + max_retries=2 > + xvfb_test=255 > + until [ $retry_count -gt $max_retries ]; do > + xvinfo || xvfb_test=$? && if [ $xvfb_test != 255 ]; then I think this handles the case where xvinfo exits with 0 incorrectly. I'm not sure if that ever happens. How about a somewhat easier to read xvinfo && xvfb_test=0 || xvfb_test=$? if [ $xvfb_test != 255 ]; then ... @@ +86,5 @@ > + max_retries=2 > + xvfb_test=255 > + until [ $retry_count -gt $max_retries ]; do > + xvinfo || xvfb_test=$? && if [ $xvfb_test != 255 ]; then > + retry_count=$(($max_retries + 1)) You could replace this with `break`, I think -- I didn't realize at first that it wasn't incrementing retry_count.

(In reply to Dustin J. Mitchell [:dustin] from comment #2) > Comment on attachment 8607616 [details] [diff] [review] > xvfb.retry.diff > > Review of attachment 8607616 [details] [diff] [review]: > ----------------------------------------------------------------- > > Perhaps xvinfo never exits 0, but just in case, this probably deserves an > update.. > > ::: testing/docker/desktop-build/bin/build.sh > @@ +85,5 @@ > > + retry_count=0 > > + max_retries=2 > > + xvfb_test=255 > > + until [ $retry_count -gt $max_retries ]; do > > + xvinfo || xvfb_test=$? && if [ $xvfb_test != 255 ]; then > > I think this handles the case where xvinfo exits with 0 incorrectly. I'm > not sure if that ever happens. How about a somewhat easier to read > > xvinfo && xvfb_test=0 || xvfb_test=$? > if [ $xvfb_test != 255 ]; then ... > > @@ +86,5 @@ > > + max_retries=2 > > + xvfb_test=255 > > + until [ $retry_count -gt $max_retries ]; do > > + xvinfo || xvfb_test=$? && if [ $xvfb_test != 255 ]; then > > + retry_count=$(($max_retries + 1)) > > You could replace this with `break`, I think -- I didn't realize at first > that it wasn't incrementing retry_count. Derp, I'm not the brightest. Why not just set the default value of xvfb_test to 0, that will cover this case I think.

I'm derpy today: how about > retry_count=0 max_retries=2 until [ $retry_count -gt $max_retries ]; do xvinfo xvfb_test=$? if [ $xvfb_test != 255 ]; then retry_count=$(($max_retries + 1)) else retry_count=$(($retry_count + 1)) echo "Failed to start Xvfb, retry: $retry_count" sleep 2 fi done if [ $xvfb_test == 255 ]; then exit 255; fi

Comment on attachment 8607756 [details] [diff] [review] xvfb.retry.diff Review of attachment 8607756 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/docker/desktop-build/bin/build.sh @@ +87,5 @@ > + xvfb_test=0 > + until [ $retry_count -gt $max_retries ]; do > + xvinfo || xvfb_test=$? > + if [ $xvfb_test != 255 ]; then > + retry_count=$(($max_retries + 1)) please at least add a comment here :)