From 389fdcf79ff4d0b5cc403f3ccda14ae0d0af0c16 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 7 Nov 2011 20:52:14 +0000 Subject: [PATCH 01/11] complete-run: pass outdir (not only logpath) to activate_i3() --- testcases/complete-run.pl | 3 ++- testcases/lib/i3test.pm | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/testcases/complete-run.pl b/testcases/complete-run.pl index e7651022..939ee636 100755 --- a/testcases/complete-run.pl +++ b/testcases/complete-run.pl @@ -141,6 +141,7 @@ sub take_job { unix_socket_path => "/tmp/nested-$display-activation", display => $display, configfile => $tmpfile, + outdir => $outdir, logpath => $logpath, cv => $activate_cv ); @@ -189,7 +190,7 @@ sub take_job { my $output; open(my $spool, '>', \$output); my $parser = TAP::Parser->new({ - exec => [ 'sh', '-c', qq|DISPLAY=$display LOGPATH="$logpath" /usr/bin/perl -Ilib $test| ], + exec => [ 'sh', '-c', qq|DISPLAY=$display LOGPATH="$logpath" OUTDIR="$outdir" /usr/bin/perl -Ilib $test| ], spool => $spool, merge => 1, }); diff --git a/testcases/lib/i3test.pm b/testcases/lib/i3test.pm index 39fd7a94..563837df 100644 --- a/testcases/lib/i3test.pm +++ b/testcases/lib/i3test.pm @@ -433,6 +433,7 @@ sub launch_with_config { unix_socket_path => "$tmp_socket_path-activation", display => $ENV{DISPLAY}, configfile => $tmpfile, + outdir => $ENV{OUTDIR}, logpath => $ENV{LOGPATH}, cv => $cv, ); From 1c0d69d4e62c71c09607b70ad2a4be2e05e1cbab Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 7 Nov 2011 20:53:49 +0000 Subject: [PATCH 02/11] complete-run: implement --valgrind --- testcases/complete-run.pl | 5 ++++- testcases/lib/SocketActivation.pm | 7 +++++++ testcases/lib/i3test.pm | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/testcases/complete-run.pl b/testcases/complete-run.pl index 939ee636..a66b0982 100755 --- a/testcases/complete-run.pl +++ b/testcases/complete-run.pl @@ -50,10 +50,12 @@ sub slurp { } my $coverage_testing = 0; +my $valgrind = 0; my @displays = (); my $result = GetOptions( "coverage-testing" => \$coverage_testing, + "valgrind" => \$valgrind, "display=s" => \@displays, ); @@ -143,6 +145,7 @@ sub take_job { configfile => $tmpfile, outdir => $outdir, logpath => $logpath, + valgrind => $valgrind, cv => $activate_cv ); @@ -190,7 +193,7 @@ sub take_job { my $output; open(my $spool, '>', \$output); my $parser = TAP::Parser->new({ - exec => [ 'sh', '-c', qq|DISPLAY=$display LOGPATH="$logpath" OUTDIR="$outdir" /usr/bin/perl -Ilib $test| ], + exec => [ 'sh', '-c', qq|DISPLAY=$display LOGPATH="$logpath" OUTDIR="$outdir" VALGRIND=$valgrind /usr/bin/perl -Ilib $test| ], spool => $spool, merge => 1, }); diff --git a/testcases/lib/SocketActivation.pm b/testcases/lib/SocketActivation.pm index 1ecfb2aa..4da2029c 100644 --- a/testcases/lib/SocketActivation.pm +++ b/testcases/lib/SocketActivation.pm @@ -78,6 +78,13 @@ sub activate_i3 { # the interactive signalhandler to make it crash immediately instead. my $i3cmd = abs_path("../i3") . " -V -d all --disable-signalhandler"; + if ($args{valgrind}) { + $i3cmd = + qq|valgrind -v --log-file="$args{outdir}/valgrind.log" | . + qq|--leak-check=full --track-origins=yes --num-callers=20 | . + qq|--tool=memcheck -- $i3cmd|; + } + # Append to $args{logpath} instead of overwriting because i3 might be # run multiple times in one testcase. my $cmd = "exec $i3cmd -c $args{configfile} >>$args{logpath} 2>&1"; diff --git a/testcases/lib/i3test.pm b/testcases/lib/i3test.pm index 563837df..7473b617 100644 --- a/testcases/lib/i3test.pm +++ b/testcases/lib/i3test.pm @@ -435,6 +435,7 @@ sub launch_with_config { configfile => $tmpfile, outdir => $ENV{OUTDIR}, logpath => $ENV{LOGPATH}, + valgrind => $ENV{VALGRIND}, cv => $cv, ); From c75cc525f7fb76945347d71216b815e72b8eba9a Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 7 Nov 2011 20:54:33 +0000 Subject: [PATCH 03/11] lib/SocketActivation: use single quotes (for consistency) --- testcases/lib/SocketActivation.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testcases/lib/SocketActivation.pm b/testcases/lib/SocketActivation.pm index 4da2029c..47a709d5 100644 --- a/testcases/lib/SocketActivation.pm +++ b/testcases/lib/SocketActivation.pm @@ -90,7 +90,7 @@ sub activate_i3 { my $cmd = "exec $i3cmd -c $args{configfile} >>$args{logpath} 2>&1"; # We need to use the shell due to using output redirections. - exec "/bin/sh", '-c', $cmd; + exec '/bin/sh', '-c', $cmd; # if we are still here, i3 could not be found or exec failed. bail out. exit 1; From dbd6440432e30e6810bede7f879d1f9414fc99a0 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 7 Nov 2011 20:56:04 +0000 Subject: [PATCH 04/11] complete-run: Use Carp::Always to get nice stacktraces in case of errors --- testcases/complete-run.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/testcases/complete-run.pl b/testcases/complete-run.pl index a66b0982..e2d8ccb8 100755 --- a/testcases/complete-run.pl +++ b/testcases/complete-run.pl @@ -13,6 +13,7 @@ use strict; use warnings; use v5.10; # the following are modules which ship with Perl (>= 5.10): +use Carp::Always; use Cwd qw(abs_path); use File::Basename qw(basename); use File::Temp qw(tempfile tempdir); From fdf7b1706c8571616c6af3c2fdc4f02c549e340d Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 7 Nov 2011 20:56:36 +0000 Subject: [PATCH 05/11] =?UTF-8?q?complete-run:=20Bugfix:=20Don=E2=80=99t?= =?UTF-8?q?=20call=20recv=20inside=20a=20callback=20when=20cleanly=20exiti?= =?UTF-8?q?ng=20i3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- testcases/complete-run.pl | 53 +++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/testcases/complete-run.pl b/testcases/complete-run.pl index e2d8ccb8..6cd00fe8 100755 --- a/testcases/complete-run.pl +++ b/testcases/complete-run.pl @@ -139,7 +139,9 @@ sub take_job { my $time_before_start = [gettimeofday]; my $pid; - if (!$dont_start) { + if ($dont_start) { + $activate_cv->send(1); + } else { $pid = activate_i3( unix_socket_path => "/tmp/nested-$display-activation", display => $display, @@ -161,20 +163,33 @@ sub take_job { # Don’t bother killing i3 when we haven’t started it return if $dont_start; + my $kill_cv = AnyEvent->condvar; + # When measuring code coverage, try to exit i3 cleanly (otherwise, .gcda # files are not written) and fallback to killing it - if ($coverage_testing) { + if ($coverage_testing || $valgrind) { my $exited = 0; - eval { - say "Exiting i3 cleanly..."; - i3("/tmp/nested-$display")->command('exit')->recv; - $exited = 1; - }; - return if $exited; + say "[$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 { + # No coverage testing or valgrind? Just kill -9 i3. + kill(9, $pid) or die "Could not kill i3 using kill($pid)"; + $kill_cv->send(); } - say "[$display] killing i3"; - kill(9, $pid) or die "could not kill i3"; + return $kill_cv; }; # This will be called as soon as i3 is running and answered to our @@ -224,21 +239,21 @@ sub take_job { $aggregator->add($test, $parser); push @done, [ $test, $output ]; - $kill_i3->(); + my $exitcv = $kill_i3->(); + $exitcv->cb(sub { - undef $_ for @watchers; - if (@done == $num) { - $cv->send; - } else { - take_job($display); - } + undef $_ for @watchers; + if (@done == $num) { + $cv->send; + } else { + take_job($display); + } + }); } ); push @watchers, $w; } }); - - $activate_cv->send(1) if $dont_start; } $cv->recv; From 0615cb35958d10304816f55b187bb2dc5fa3cf49 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 7 Nov 2011 21:21:51 +0000 Subject: [PATCH 06/11] complete-run.pl: implement --help --- testcases/complete-run.pl | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/testcases/complete-run.pl b/testcases/complete-run.pl index 6cd00fe8..adef4837 100755 --- a/testcases/complete-run.pl +++ b/testcases/complete-run.pl @@ -13,6 +13,7 @@ use strict; use warnings; use v5.10; # the following are modules which ship with Perl (>= 5.10): +use Pod::Usage; use Carp::Always; use Cwd qw(abs_path); use File::Basename qw(basename); @@ -52,14 +53,18 @@ sub slurp { my $coverage_testing = 0; my $valgrind = 0; +my $help = 0; my @displays = (); my $result = GetOptions( "coverage-testing" => \$coverage_testing, "valgrind" => \$valgrind, "display=s" => \@displays, + "help|?" => \$help, ); +pod2usage(0) if $help; + @displays = split(/,/, join(',', @displays)); @displays = map { s/ //g; $_ } @displays; @@ -268,3 +273,37 @@ for (@done) { # 4: print summary $harness->summary($aggregator); + +__END__ + +=head1 NAME + +complete-run.pl - Run the i3 testsuite + +=head1 SYNOPSIS + +complete-run.pl [files...] + +=head1 OPTIONS + +=over 8 + +=item B<--display> + +Specifies which X11 display should be used. Can be specified multiple times and +will parallelize the tests: + + # Run tests on the second X server + ./complete-run.pl -d :1 + + # Run four tests in parallel on some Xdummy servers + ./complete-run.pl -d :1,:2,:3,:4 + +=item B<--valgrind> + +Runs i3 under valgrind to find memory problems. The output will be available in +C. + +=item B<--coverage-testing> + +Exits i3 cleanly (instead of kill -9) to make coverage testing work properly. From 2a78a5f2b6c8c011dfcd0e794a57728d6ed0fb71 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 7 Nov 2011 21:34:39 +0000 Subject: [PATCH 07/11] ipc: fix memory leaks when clients disconnect --- src/ipc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ipc.c b/src/ipc.c index b94ef613..0a738427 100644 --- a/src/ipc.c +++ b/src/ipc.c @@ -99,9 +99,12 @@ void ipc_send_event(const char *event, uint32_t message_type, const char *payloa */ void ipc_shutdown() { ipc_client *current; - TAILQ_FOREACH(current, &all_clients, clients) { + while (!TAILQ_EMPTY(&all_clients)) { + current = TAILQ_FIRST(&all_clients); shutdown(current->fd, SHUT_RDWR); close(current->fd); + TAILQ_REMOVE(&all_clients, current, clients); + free(current); } } @@ -743,10 +746,12 @@ static void ipc_receive_message(EV_P_ struct ev_io *w, int revents) { /* We can call TAILQ_REMOVE because we break out of the * TAILQ_FOREACH afterwards */ TAILQ_REMOVE(&all_clients, current, clients); + free(current); break; } ev_io_stop(EV_A_ w); + free(w); DLOG("IPC: client disconnected\n"); return; From f0cc13f356672cd4e5e328e11db01324d943cf9f Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 7 Nov 2011 21:34:50 +0000 Subject: [PATCH 08/11] i3bar: fix indention in src/child.c --- i3bar/src/child.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/i3bar/src/child.c b/i3bar/src/child.c index 44e237f8..e294fb9a 100644 --- a/i3bar/src/child.c +++ b/i3bar/src/child.c @@ -40,8 +40,8 @@ void cleanup() { ev_io_stop(main_loop, stdin_io); FREE(stdin_io); FREE(statusline_buffer); - /* statusline pointed to memory within statusline_buffer */ - statusline = NULL; + /* statusline pointed to memory within statusline_buffer */ + statusline = NULL; } if (child_sig != NULL) { From 8b887e8447c95b226d214bfe74cbc80137f297ab Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 7 Nov 2011 22:38:06 +0000 Subject: [PATCH 09/11] complete-run: Bugfix: return condvar when $dont_start is true --- testcases/complete-run.pl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/testcases/complete-run.pl b/testcases/complete-run.pl index adef4837..2f05f703 100755 --- a/testcases/complete-run.pl +++ b/testcases/complete-run.pl @@ -165,11 +165,14 @@ sub take_job { } my $kill_i3 = sub { - # Don’t bother killing i3 when we haven’t started it - return if $dont_start; - 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) { From afc488021f9ca204cfb2e8c462b743fde33f70e2 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 7 Nov 2011 23:04:45 +0000 Subject: [PATCH 10/11] complete-run.pl: automatically start Xdummy instances unless -d is specified This makes running the testsuite incredibly easy: $ ./complete-run.pl :) --- testcases/complete-run.pl | 52 ++++++++++++++++---- testcases/lib/SocketActivation.pm | 2 + testcases/lib/StartXDummy.pm | 80 +++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 10 deletions(-) create mode 100644 testcases/lib/StartXDummy.pm diff --git a/testcases/complete-run.pl b/testcases/complete-run.pl index 2f05f703..915a7e02 100755 --- a/testcases/complete-run.pl +++ b/testcases/complete-run.pl @@ -1,13 +1,6 @@ #!/usr/bin/env perl # vim:ts=4:sw=4:expandtab -# # © 2010-2011 Michael Stapelberg and contributors -# -# syntax: ./complete-run.pl --display :1 --display :2 -# to run the test suite on the X11 displays :1 and :2 -# use 'Xdummy :1' and 'Xdummy :2' before to start two -# headless X11 servers -# use strict; use warnings; @@ -28,6 +21,7 @@ use TAP::Parser::Aggregator; # these are shipped with the testsuite use lib qw(lib); use SocketActivation; +use StartXDummy; # the following modules are not shipped with Perl use AnyEvent; use AnyEvent::Handle; @@ -46,7 +40,7 @@ $SIG{CHLD} = sub { # reads in a whole file sub slurp { - open my $fh, '<', shift; + open(my $fh, '<', shift); local $/; <$fh>; } @@ -54,21 +48,32 @@ sub slurp { my $coverage_testing = 0; my $valgrind = 0; my $help = 0; +# Number of tests to run in parallel. Important to know how many Xdummy +# instances we need to start (unless @displays are given). Defaults to +# num_cores * 2. +my $parallel = undef; my @displays = (); +my @childpids = (); my $result = GetOptions( "coverage-testing" => \$coverage_testing, "valgrind" => \$valgrind, "display=s" => \@displays, + "parallel=i" => \$parallel, "help|?" => \$help, ); -pod2usage(0) if $help; +pod2usage(-verbose => 2, -exitcode => 0) if $help; @displays = split(/,/, join(',', @displays)); @displays = map { s/ //g; $_ } @displays; -@displays = qw(:1) if @displays == 0; +# No displays specified, let’s start some Xdummy instances. +if (@displays == 0) { + my ($displays, $pids) = start_xdummy($parallel); + @displays = @$displays; + @childpids = @$pids; +} # connect to all displays for two reasons: # 1: check if the display actually works @@ -88,6 +93,8 @@ 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 @@ -268,6 +275,9 @@ $cv->recv; $aggregator->stop(); +# Disable buffering to make sure the output and summary appear before we exit. +$| = 1; + for (@done) { my ($test, $output) = @$_; say "output for $test:"; @@ -277,6 +287,8 @@ for (@done) { # 4: print summary $harness->summary($aggregator); +kill(15, $_) for @childpids; + __END__ =head1 NAME @@ -287,6 +299,15 @@ complete-run.pl - Run the i3 testsuite complete-run.pl [files...] +=head1 EXAMPLE + +To run the whole testsuite on a reasonable number of Xdummy instances (your +running X11 will not be touched), run: + ./complete-run.pl + +To run only a specific test (useful when developing a new feature), run: + ./complete-run t/100-fullscreen.t + =head1 OPTIONS =over 8 @@ -302,6 +323,9 @@ will parallelize the tests: # Run four tests in parallel on some Xdummy servers ./complete-run.pl -d :1,:2,:3,:4 +Note that it is not necessary to specify this anymore. If omitted, +complete-run.pl will start (num_cores * 2) Xdummy instances. + =item B<--valgrind> Runs i3 under valgrind to find memory problems. The output will be available in @@ -310,3 +334,11 @@ C. =item B<--coverage-testing> Exits i3 cleanly (instead of kill -9) to make coverage testing work properly. + +=item B<--parallel> + +Number of Xdummy instances to start (if you don’t want to start num_cores * 2 +instances for some reason). + + # Run all tests on a single Xdummy instance + ./complete-run.pl -p 1 diff --git a/testcases/lib/SocketActivation.pm b/testcases/lib/SocketActivation.pm index 47a709d5..31b3ba89 100644 --- a/testcases/lib/SocketActivation.pm +++ b/testcases/lib/SocketActivation.pm @@ -1,6 +1,8 @@ package SocketActivation; # vim:ts=4:sw=4:expandtab +use strict; +use warnings; use IO::Socket::UNIX; # core use Cwd qw(abs_path); # core use POSIX; # core diff --git a/testcases/lib/StartXDummy.pm b/testcases/lib/StartXDummy.pm new file mode 100644 index 00000000..8657ba9d --- /dev/null +++ b/testcases/lib/StartXDummy.pm @@ -0,0 +1,80 @@ +package StartXDummy; +# vim:ts=4:sw=4:expandtab + +use strict; +use warnings; +use POSIX (); +use Exporter 'import'; +use Time::HiRes qw(sleep); +use v5.10; + +our @EXPORT = qw(start_xdummy); + +# reads in a whole file +sub slurp { + open(my $fh, '<', shift) or return ''; + local $/; + <$fh>; +} + +=head2 start_xdummy($parallel) + +Starts C<$parallel> (or number of cores * 2 if undef) Xdummy processes (see +the file ./Xdummy) and returns two arrayrefs: a list of X11 display numbers to +the Xdummy processes and a list of PIDs of the processes. + +=cut +sub start_xdummy { + my ($parallel) = @_; + + my @displays = (); + my @childpids = (); + + # Yeah, I know it’s non-standard, but Perl’s POSIX module doesn’t have + # _SC_NPROCESSORS_CONF. + my $cpuinfo = slurp('/proc/cpuinfo'); + my $num_cores = scalar grep { /model name/ } split("\n", $cpuinfo); + # If /proc/cpuinfo does not exist, we fall back to 2 cores. + $num_cores ||= 2; + + $parallel ||= $num_cores * 2; + + # First get the last used display number, then increment it by one. + # Effectively falls back to 1 if no X server is running. + my ($displaynum) = reverse ('0', sort ); + $displaynum =~ s/.*(\d)$/$1/; + $displaynum++; + + say "Starting $parallel Xdummy instances, starting at :$displaynum..."; + + for my $idx (0 .. ($parallel-1)) { + my $pid = fork(); + die "Could not fork: $!" unless defined($pid); + if ($pid == 0) { + # Child, close stdout/stderr, then start Xdummy. + POSIX::close(0); + POSIX::close(2); + exec './Xdummy', ":$displaynum", '-config', '/dev/null'; + exit 1; + } + push(@childpids, $pid); + push(@displays, ":$displaynum"); + $displaynum++; + } + + # Wait until the X11 sockets actually appear. Pretty ugly solution, but as + # long as we can’t socket-activate X11… + my $sockets_ready; + do { + $sockets_ready = 1; + for (@displays) { + my $path = "/tmp/.X11-unix/X" . substr($_, 1); + $sockets_ready = 0 unless -S $path; + } + sleep 0.1; + } until $sockets_ready; + + return \@displays, \@childpids; +} + +1 From bf12befd6dd54c6cb0a66d6355acc35f4a783d93 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 7 Nov 2011 23:07:08 +0000 Subject: [PATCH 11/11] lib/StartXDummy.pm: document why we use -config /dev/null --- testcases/lib/StartXDummy.pm | 3 +++ 1 file changed, 3 insertions(+) diff --git a/testcases/lib/StartXDummy.pm b/testcases/lib/StartXDummy.pm index 8657ba9d..36b9e006 100644 --- a/testcases/lib/StartXDummy.pm +++ b/testcases/lib/StartXDummy.pm @@ -54,6 +54,9 @@ sub start_xdummy { # Child, close stdout/stderr, then start Xdummy. POSIX::close(0); POSIX::close(2); + # We use -config /dev/null to prevent Xdummy from using the system + # Xorg configuration. The tests should be independant from the + # actual system X configuration. exec './Xdummy', ":$displaynum", '-config', '/dev/null'; exit 1; }