testcases: always start i3 through i3test::launch_with_config

this will pave the way to just call BAIL_OUT and stop the currently
running testfile if i3 died for whatever reason.

testcases/complete-run.pl

@@ -19,7 +19,6 @@ use TAP::Parser;
 use TAP::Parser::Aggregator;
 # these are shipped with the testsuite
 use lib qw(lib);
-use SocketActivation;
 use StartXDummy;
 use StatusLine;
 # the following modules are not shipped with Perl
@@ -43,13 +42,6 @@ my $cv = AnyEvent->condvar;
 $SIG{CHLD} = sub {
 };
 
-# reads in a whole file
-sub slurp {
-    open(my $fh, '<', shift);
-    local $/;
-    <$fh>;
-}
-
 # convinience wrapper to write to the log file
 my $log;
 sub Log { say $log "@_" }
@@ -106,8 +98,6 @@ for my $display (@displays) {
 
 die "No usable displays found" if @wdisplays == 0;
 
-my $config = slurp('i3-test.config');
-
 # 1: get a list of all testcases
 my @testfiles = @ARGV;
 
@@ -157,97 +147,14 @@ sub take_job {
     my $test = shift @testfiles
         or return $cv->end;
 
-    my $dont_start = (slurp($test) =~ /# !NO_I3_INSTANCE!/);
     my $basename = basename($test);
-    my $logpath = "$outdir/i3-log-for-$basename";
-
-    my ($fh, $tmpfile) = tempfile("i3-cfg-for-$basename.XXXXXX", UNLINK => 1);
-    say $fh $config;
-    say $fh "ipc-socket /tmp/nested-$display";
-    close($fh);
-
-    my $activate_cv = AnyEvent->condvar;
-    my $time_before_start = [gettimeofday];
-
-    my $pid;
-    if ($dont_start) {
-        $activate_cv->send(1);
-    } else {
-        $pid = activate_i3(
-            unix_socket_path => "/tmp/nested-$display-activation",
-            display => $display,
-            configfile => $tmpfile,
-            outdir => $outdir,
-            testname => $basename,
-            valgrind => $valgrind,
-            strace => $strace,
-            cv => $activate_cv
-        );
-
-        my $child_watcher;
-        $child_watcher = AnyEvent->child(pid => $pid, cb => sub {
-            Log status($display, "child died. pid = $pid");
-            undef $child_watcher;
-        });
-    }
-
-    my $kill_i3 = sub {
-        my $kill_cv = AnyEvent->condvar;
-
-        # Don’t bother killing i3 when we haven’t started it
-        if ($dont_start) {
-            $kill_cv->send();
-            return $kill_cv;
-        }
-
-        # When measuring code coverage, try to exit i3 cleanly (otherwise, .gcda
-        # files are not written) and fallback to killing it
-        if ($coverage_testing || $valgrind) {
-            my $exited = 0;
-            Log status($display, 'Exiting i3 cleanly...');
-            my $i3 = i3("/tmp/nested-$display");
-            $i3->connect->cb(sub {
-                if (!$_[0]->recv) {
-                    # Could not connect to i3, just kill -9 it
-                    kill(9, $pid) or die "Could not kill i3 using kill($pid)";
-                    $kill_cv->send();
-                } else {
-                    # Connected. Now send exit and continue once that’s acked.
-                    $i3->command('exit')->cb(sub {
-                        $kill_cv->send();
-                    });
-                }
-            });
-        } else {
-            Log status($display, 'killing i3');
-
-            # No coverage testing or valgrind? Just kill -9 i3.
-            kill(9, $pid) or die "Could not kill i3 using kill($pid)";
-            $kill_cv->send();
-        }
-
-        return $kill_cv;
-    };
-
-    # This will be called as soon as i3 is running and answered to our
-    # IPC request
-    $activate_cv->cb(sub {
-        my $time_activating = [gettimeofday];
-        my $start_duration = tv_interval($time_before_start, $time_activating);
-        my ($status) = $activate_cv->recv;
-        if ($dont_start) {
-            Log status($display, 'Not starting i3, testcase does that');
-        } else {
-            my $duration = sprintf("%.2f", $start_duration);
-            Log status($display, "i3 startup: took $duration sec, status = $status");
-        }
 
     Log status($display, "Starting $test");
 
     my $output;
     open(my $spool, '>', \$output);
     my $parser = TAP::Parser->new({
-            exec => [ 'sh', '-c', qq|DISPLAY=$display TESTNAME="$basename" OUTDIR="$outdir" VALGRIND=$valgrind STRACE=$strace /usr/bin/perl -Ilib $test| ],
+        exec => [ 'sh', '-c', qq|DISPLAY=$display TESTNAME="$basename" OUTDIR="$outdir" VALGRIND=$valgrind STRACE=$strace COVERAGE=$coverage_testing /usr/bin/perl -Ilib $test| ],
         spool => $spool,
         merge => 1,
     });
@@ -283,21 +190,16 @@ sub take_job {
 
                 status_completed(scalar @done);
 
-                    my $exitcv = $kill_i3->();
-                    $exitcv->cb(sub {
-
                 undef $_ for @watchers;
                 if (@done == $num) {
                     $cv->end;
                 } else {
                     take_job($display);
                 }
-                    });
             }
         );
         push @watchers, $w;
     }
-    });
 }
 
 $cv->recv;

testcases/lib/i3test.pm

@@ -56,11 +56,54 @@ BEGIN {
     }
 }
 
+my $i3_pid;
+my $i3_autostart;
+
+END {
+
+    # testcases which start i3 manually should always call exit_gracefully
+    # on their own. Let’s see, whether they really did.
+    if (! $i3_autostart) {
+        return unless $i3_pid;
+
+        $tester->ok(undef, 'testcase called exit_gracefully()');
+    }
+
+    # don't trigger SIGCHLD handler
+    local $SIG{CHLD};
+
+    # From perldoc -v '$?':
+    # Inside an "END" subroutine $? contains the value
+    # that is going to be given to "exit()".
+    #
+    # Since waitpid sets $?, we need to localize it,
+    # otherwise TAP would be misinterpreted our return status
+    local $?;
+
+    # When measuring code coverage, try to exit i3 cleanly (otherwise, .gcda
+    # files are not written)
+    if ($ENV{COVERAGE} || $ENV{VALGRIND}) {
+        exit_gracefully($i3_pid, "/tmp/nested-$ENV{DISPLAY}");
+
+    } else {
+        kill(9, $i3_pid)
+            or $tester->BAIL_OUT("could not kill i3");
+
+        waitpid $i3_pid, 0;
+    }
+}
+
 sub import {
-    my $class = shift;
+    my ($class, %args) = @_;
     my $pkg = caller;
 
-    my $test_more_args = @_ ? "qw(@_)" : "";
+    $i3_autostart = delete($args{i3_autostart}) // 1;
+
+    my $cv = launch_with_config('-default', dont_block => 1)
+        if $i3_autostart;
+
+    my $test_more_args = '';
+    $test_more_args = join(' ', 'qw(', %args, ')') if keys %args;
     local $@;
     eval << "__";
 package $pkg;
@@ -69,12 +112,14 @@ use Data::Dumper;
 use AnyEvent::I3;
 use Time::HiRes qw(sleep);
 __
-    $tester->bail_out("$@") if $@;
+    $tester->BAIL_OUT("$@") if $@;
     feature->import(":5.10");
     strict->import;
     warnings->import;
 
     $x ||= i3test::X11->new;
+    $cv->recv if $i3_autostart;
+
     @_ = ($class);
     goto \&Exporter::import;
 }
@@ -394,12 +439,16 @@ sub exit_gracefully {
     };
 
     if (!$exited) {
-        kill(9, $pid) or die "could not kill i3";
+        kill(9, $pid)
+            or $tester->BAIL_OUT("could not kill i3");
     }
 
     if ($socketpath =~ m,^/tmp/i3-test-socket-,) {
         unlink($socketpath);
     }
+
+    waitpid $pid, 0;
+    undef $i3_pid;
 }
 
 # Gets the socket path from the I3_SOCKET_PATH atom stored on the X11 root window
@@ -427,21 +476,28 @@ sub get_socket_path {
 # complete-run.pl that it should not create an instance of i3
 #
 sub launch_with_config {
-    my ($config, $dont_add_socket_path) = @_;
+    my ($config, %args) = @_;
 
-    $dont_add_socket_path //= 0;
+    $tmp_socket_path = "/tmp/nested-$ENV{DISPLAY}";
 
-    if (!defined($tmp_socket_path)) {
+    my ($fh, $tmpfile) = tempfile("i3-cfg-for-$ENV{TESTNAME}-XXXXX", UNLINK => 1);
-        $tmp_socket_path = File::Temp::tempnam('/tmp', 'i3-test-socket-');
+
+    if ($config ne '-default') {
+        say $fh $config;
+    } else {
+        open(my $conf_fh, '<', './i3-test.config')
+            or $tester->BAIL_OUT("could not open default config: $!");
+        local $/;
+        say $fh scalar <$conf_fh>;
     }
 
-    my ($fh, $tmpfile) = tempfile('/tmp/i3-test-config-XXXXX', UNLINK => 1);
+    say $fh "ipc-socket $tmp_socket_path"
-    say $fh $config;
+        unless $args{dont_add_socket_path};
-    say $fh "ipc-socket $tmp_socket_path" unless $dont_add_socket_path;
+
     close($fh);
 
     my $cv = AnyEvent->condvar;
-    my $pid = activate_i3(
+    $i3_pid = activate_i3(
         unix_socket_path => "$tmp_socket_path-activation",
         display => $ENV{DISPLAY},
         configfile => $tmpfile,
@@ -452,13 +508,16 @@ sub launch_with_config {
         cv => $cv,
     );
 
+    # force update of the cached socket path in lib/i3test
+    # as soon as i3 has started
+    $cv->cb(sub { get_socket_path(0) });
+
+    return $cv if $args{dont_block};
+
     # blockingly wait until i3 is ready
     $cv->recv;
 
-    # force update of the cached socket path in lib/i3test
+    return $i3_pid;
-    get_socket_path(0);
-
-    return $pid;
 }
 
 package i3test::X11;

testcases/t/159-socketpaths.t

@@ -1,10 +1,9 @@
 #!perl
 # vim:ts=4:sw=4:expandtab
-# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
 #
 # Tests if the various ipc_socket_path options are correctly handled
 #
-use i3test;
+use i3test i3_autostart => 0;
 use File::Temp qw(tempfile tempdir);
 use POSIX qw(getuid);
 use v5.10;
@@ -20,7 +19,7 @@ EOT
 
 # ensure XDG_RUNTIME_DIR is not set
 delete $ENV{XDG_RUNTIME_DIR};
-my $pid = launch_with_config($config, 1);
+my $pid = launch_with_config($config, dont_add_socket_path => 1);
 
 my $folder = "/tmp/i3-" . getpwuid(getuid());
 ok(-d $folder, "folder $folder exists");
@@ -39,7 +38,7 @@ ok(! -e "$rtdir/i3", "$rtdir/i3 does not exist yet");
 
 $ENV{XDG_RUNTIME_DIR} = $rtdir;
 
-$pid = launch_with_config($config, 1);
+$pid = launch_with_config($config, dont_add_socket_path => 1);
 
 ok(-d "$rtdir/i3", "$rtdir/i3 exists and is a directory");
 $socketpath = "$rtdir/i3/ipc-socket." . $pid;
@@ -61,7 +60,7 @@ font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
 ipc-socket $socketpath
 EOT
 
-$pid = launch_with_config($config, 1);
+$pid = launch_with_config($config, dont_add_socket_path => 1);
 
 ok(-S $socketpath, "file $socketpath exists and is a socket");
 

testcases/t/165-for_window.t

@@ -1,9 +1,7 @@
 #!perl
 # vim:ts=4:sw=4:expandtab
-# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
 #
-#
+use i3test i3_autostart => 0;
-use i3test;
 use X11::XCB qw(PROP_MODE_REPLACE);
 
 ##############################################################

testcases/t/166-assign.t

@@ -1,10 +1,9 @@
 #!perl
 # vim:ts=4:sw=4:expandtab
-# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
 #
 # Tests if assignments work
 #
-use i3test;
+use i3test i3_autostart => 0;
 use X11::XCB qw(PROP_MODE_REPLACE);
 
 # TODO: move to X11::XCB

testcases/t/167-workspace_layout.t

@@ -1,11 +1,10 @@
 #!perl
 # vim:ts=4:sw=4:expandtab
-# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
 #
 # Tests the workspace_layout config option.
 #
 
-use i3test;
+use i3test i3_autostart => 0;
 
 #####################################################################
 # 1: check that with an empty config, cons are place next to each

testcases/t/170-force_focus_wrapping.t

@@ -1,10 +1,9 @@
 #!perl
 # vim:ts=4:sw=4:expandtab
-# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
 #
 # Tests if the 'force_focus_wrapping' config directive works correctly.
 #
-use i3test;
+use i3test i3_autostart => 0;
 
 #####################################################################
 # 1: test the wrapping behaviour without force_focus_wrapping

testcases/t/171-config-migrate.t

@@ -1,11 +1,10 @@
 #!perl
 # vim:ts=4:sw=4:expandtab
-# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
 #
 # Tests if i3-migrate-config-to-v4 correctly migrates all config file
 # directives and commands
 #
-use i3test;
+use i3test i3_autostart => 0;
 use Cwd qw(abs_path);
 use File::Temp qw(tempfile tempdir);
 use v5.10;

testcases/t/172-start-on-named-ws.t

@@ -1,10 +1,8 @@
 #!perl
 # vim:ts=4:sw=4:expandtab
-# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
-#
 # checks if i3 starts up on workspace '1' or the first configured named workspace
 #
-use i3test;
+use i3test i3_autostart => 0;
 
 ##############################################################
 # 1: i3 should start with workspace '1'

testcases/t/173-regress-focus-assign.t

@@ -1,11 +1,10 @@
 #!perl
 # vim:ts=4:sw=4:expandtab
-# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
 #
 # Regression: Checks if focus is stolen when a window is managed which is
 # assigned to an invisible workspace
 #
-use i3test;
+use i3test i3_autostart => 0;
 use X11::XCB qw(PROP_MODE_REPLACE);
 
 # TODO: move to X11::XCB

testcases/t/174-border-config.t

@@ -1,11 +1,10 @@
 #!perl
 # vim:ts=4:sw=4:expandtab
-# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
 #
 # Tests the new_window and new_float config option.
 #
 
-use i3test;
+use i3test i3_autostart => 0;
 
 #####################################################################
 # 1: check that new windows start with 'normal' border unless configured

testcases/t/176-workspace-baf.t

@@ -1,12 +1,10 @@
 #!perl
 # vim:ts=4:sw=4:expandtab
-# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
-#
 # Checks if the 'workspace back_and_forth' command and the
 # 'workspace_auto_back_and_forth' config directive work correctly.
 #
 
-use i3test;
+use i3test i3_autostart => 0;
 
 my $config = <<EOT;
 # i3 config file (v4)

testcases/t/177-bar-config.t

@@ -1,11 +1,10 @@
 #!perl
 # vim:ts=4:sw=4:expandtab
-# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
 #
 # Checks that the bar config is parsed correctly.
 #
 
-use i3test;
+use i3test i3_autostart => 0;
 
 #####################################################################
 # test a config without any bars