From cc4e48fc1eafdb9bec037f10c22708a26fd25ef6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 17 Oct 2010 02:36:56 +0800 Subject: tests: factor out terminal handling from t7006 Other tests besides the pager ones may want to check how we handle output to a terminal. This patch makes the code reusable. Signed-off-by: Jeff King Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh new file mode 100644 index 0000000..6fc33db --- /dev/null +++ b/t/lib-terminal.sh @@ -0,0 +1,28 @@ +#!/bin/sh + +test_expect_success 'set up terminal for tests' ' + if test -t 1 + then + >stdout_is_tty + elif + test_have_prereq PERL && + "$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl \ + sh -c "test -t 1" + then + >test_terminal_works + fi +' + +if test -e stdout_is_tty +then + test_terminal() { "$@"; } + test_set_prereq TTY +elif test -e test_terminal_works +then + test_terminal() { + "$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl "$@" + } + test_set_prereq TTY +else + say "# no usable terminal, so skipping some tests" +fi diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index fb744e3..17e54d3 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -4,42 +4,13 @@ test_description='Test automatic use of a pager.' . ./test-lib.sh . "$TEST_DIRECTORY"/lib-pager.sh +. "$TEST_DIRECTORY"/lib-terminal.sh cleanup_fail() { echo >&2 cleanup failed (exit 1) } -test_expect_success 'set up terminal for tests' ' - rm -f stdout_is_tty || - cleanup_fail && - - if test -t 1 - then - >stdout_is_tty - elif - test_have_prereq PERL && - "$PERL_PATH" "$TEST_DIRECTORY"/t7006/test-terminal.perl \ - sh -c "test -t 1" - then - >test_terminal_works - fi -' - -if test -e stdout_is_tty -then - test_terminal() { "$@"; } - test_set_prereq TTY -elif test -e test_terminal_works -then - test_terminal() { - "$PERL_PATH" "$TEST_DIRECTORY"/t7006/test-terminal.perl "$@" - } - test_set_prereq TTY -else - say "# no usable terminal, so skipping some tests" -fi - test_expect_success 'setup' ' unset GIT_PAGER GIT_PAGER_IN_USE; test_might_fail git config --unset core.pager && diff --git a/t/t7006/test-terminal.perl b/t/t7006/test-terminal.perl deleted file mode 100755 index 73ff809..0000000 --- a/t/t7006/test-terminal.perl +++ /dev/null @@ -1,58 +0,0 @@ -#!/usr/bin/perl -use strict; -use warnings; -use IO::Pty; -use File::Copy; - -# Run @$argv in the background with stdout redirected to $out. -sub start_child { - my ($argv, $out) = @_; - my $pid = fork; - if (not defined $pid) { - die "fork failed: $!" - } elsif ($pid == 0) { - open STDOUT, ">&", $out; - close $out; - exec(@$argv) or die "cannot exec '$argv->[0]': $!" - } - return $pid; -} - -# Wait for $pid to finish. -sub finish_child { - # Simplified from wait_or_whine() in run-command.c. - my ($pid) = @_; - - my $waiting = waitpid($pid, 0); - if ($waiting < 0) { - die "waitpid failed: $!"; - } elsif ($? & 127) { - my $code = $? & 127; - warn "died of signal $code"; - return $code - 128; - } else { - return $? >> 8; - } -} - -sub xsendfile { - my ($out, $in) = @_; - - # Note: the real sendfile() cannot read from a terminal. - - # It is unspecified by POSIX whether reads - # from a disconnected terminal will return - # EIO (as in AIX 4.x, IRIX, and Linux) or - # end-of-file. Either is fine. - copy($in, $out, 4096) or $!{EIO} or die "cannot copy from child: $!"; -} - -if ($#ARGV < 1) { - die "usage: test-terminal program args"; -} -my $master = new IO::Pty; -my $slave = $master->slave; -my $pid = start_child(\@ARGV, $slave); -close $slave; -xsendfile(\*STDOUT, $master); -exit(finish_child($pid)); diff --git a/t/test-terminal.perl b/t/test-terminal.perl new file mode 100755 index 0000000..73ff809 --- /dev/null +++ b/t/test-terminal.perl @@ -0,0 +1,58 @@ +#!/usr/bin/perl +use strict; +use warnings; +use IO::Pty; +use File::Copy; + +# Run @$argv in the background with stdout redirected to $out. +sub start_child { + my ($argv, $out) = @_; + my $pid = fork; + if (not defined $pid) { + die "fork failed: $!" + } elsif ($pid == 0) { + open STDOUT, ">&", $out; + close $out; + exec(@$argv) or die "cannot exec '$argv->[0]': $!" + } + return $pid; +} + +# Wait for $pid to finish. +sub finish_child { + # Simplified from wait_or_whine() in run-command.c. + my ($pid) = @_; + + my $waiting = waitpid($pid, 0); + if ($waiting < 0) { + die "waitpid failed: $!"; + } elsif ($? & 127) { + my $code = $? & 127; + warn "died of signal $code"; + return $code - 128; + } else { + return $? >> 8; + } +} + +sub xsendfile { + my ($out, $in) = @_; + + # Note: the real sendfile() cannot read from a terminal. + + # It is unspecified by POSIX whether reads + # from a disconnected terminal will return + # EIO (as in AIX 4.x, IRIX, and Linux) or + # end-of-file. Either is fine. + copy($in, $out, 4096) or $!{EIO} or die "cannot copy from child: $!"; +} + +if ($#ARGV < 1) { + die "usage: test-terminal program args"; +} +my $master = new IO::Pty; +my $slave = $master->slave; +my $pid = start_child(\@ARGV, $slave); +close $slave; +xsendfile(\*STDOUT, $master); +exit(finish_child($pid)); -- cgit v0.10.2-6-g49f6 From e23f436c35c3e7fd12feeed783064e08aaf27869 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 17 Oct 2010 02:36:57 +0800 Subject: tests: test terminal output to both stdout and stderr Some outputs (like the pager) care whether stdout is a terminal. Others (like progress meters) care about stderr. This patch sets up both. Technically speaking, we could go further and set up just one (because either the other goes to a terminal, or because our tests are only interested in one). This patch does both to keep the interface to lib-terminal simple. Signed-off-by: Jeff King Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh index 6fc33db..3258b8f 100644 --- a/t/lib-terminal.sh +++ b/t/lib-terminal.sh @@ -1,19 +1,19 @@ #!/bin/sh test_expect_success 'set up terminal for tests' ' - if test -t 1 + if test -t 1 && test -t 2 then - >stdout_is_tty + >have_tty elif test_have_prereq PERL && "$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl \ - sh -c "test -t 1" + sh -c "test -t 1 && test -t 2" then >test_terminal_works fi ' -if test -e stdout_is_tty +if test -e have_tty then test_terminal() { "$@"; } test_set_prereq TTY diff --git a/t/test-terminal.perl b/t/test-terminal.perl index 73ff809..c2e9dac 100755 --- a/t/test-terminal.perl +++ b/t/test-terminal.perl @@ -4,14 +4,15 @@ use warnings; use IO::Pty; use File::Copy; -# Run @$argv in the background with stdout redirected to $out. +# Run @$argv in the background with stdio redirected to $out and $err. sub start_child { - my ($argv, $out) = @_; + my ($argv, $out, $err) = @_; my $pid = fork; if (not defined $pid) { die "fork failed: $!" } elsif ($pid == 0) { open STDOUT, ">&", $out; + open STDERR, ">&", $err; close $out; exec(@$argv) or die "cannot exec '$argv->[0]': $!" } @@ -47,12 +48,28 @@ sub xsendfile { copy($in, $out, 4096) or $!{EIO} or die "cannot copy from child: $!"; } +sub copy_stdio { + my ($out, $err) = @_; + my $pid = fork; + defined $pid or die "fork failed: $!"; + if (!$pid) { + close($out); + xsendfile(\*STDERR, $err); + exit 0; + } + close($err); + xsendfile(\*STDOUT, $out); + finish_child($pid) == 0 + or exit 1; +} + if ($#ARGV < 1) { die "usage: test-terminal program args"; } -my $master = new IO::Pty; -my $slave = $master->slave; -my $pid = start_child(\@ARGV, $slave); -close $slave; -xsendfile(\*STDOUT, $master); +my $master_out = new IO::Pty; +my $master_err = new IO::Pty; +my $pid = start_child(\@ARGV, $master_out->slave, $master_err->slave); +close $master_out->slave; +close $master_err->slave; +copy_stdio($master_out, $master_err); exit(finish_child($pid)); -- cgit v0.10.2-6-g49f6 From 05236a5e9d29a2628a8f1de96a2a256185687e6c Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sun, 17 Oct 2010 02:36:58 +0800 Subject: test-lib: allow test code to check the list of declared prerequisites This is plumbing to prepare helpers like test_terminal to notice buggy test scripts that do not declare all of the necessary prerequisites. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano diff --git a/t/test-lib.sh b/t/test-lib.sh index 2af8f10..31366e1 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -362,6 +362,15 @@ test_have_prereq () { test $total_prereq = $ok_prereq } +test_declared_prereq () { + case ",$test_prereq," in + *,$1,*) + return 0 + ;; + esac + return 1 +} + # You are not expected to call test_ok_ and test_failure_ directly, use # the text_expect_* functions instead. @@ -414,17 +423,17 @@ test_skip () { break esac done - if test -z "$to_skip" && test -n "$prereq" && - ! test_have_prereq "$prereq" + if test -z "$to_skip" && test -n "$test_prereq" && + ! test_have_prereq "$test_prereq" then to_skip=t fi case "$to_skip" in t) of_prereq= - if test "$missing_prereq" != "$prereq" + if test "$missing_prereq" != "$test_prereq" then - of_prereq=" of $prereq" + of_prereq=" of $test_prereq" fi say_color skip >&3 "skipping test: $@" @@ -438,9 +447,10 @@ test_skip () { } test_expect_failure () { - test "$#" = 3 && { prereq=$1; shift; } || prereq= + test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq= test "$#" = 2 || error "bug in the test script: not 2 or 3 parameters to test-expect-failure" + export test_prereq if ! test_skip "$@" then say >&3 "checking known breakage: $2" @@ -456,9 +466,10 @@ test_expect_failure () { } test_expect_success () { - test "$#" = 3 && { prereq=$1; shift; } || prereq= + test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq= test "$#" = 2 || error "bug in the test script: not 2 or 3 parameters to test-expect-success" + export test_prereq if ! test_skip "$@" then say >&3 "expecting success: $2" @@ -500,11 +511,12 @@ test_expect_code () { # Usage: test_external description command arguments... # Example: test_external 'Perl API' perl ../path/to/test.pl test_external () { - test "$#" = 4 && { prereq=$1; shift; } || prereq= + test "$#" = 4 && { test_prereq=$1; shift; } || test_prereq= test "$#" = 3 || error >&5 "bug in the test script: not 3 or 4 parameters to test_external" descr="$1" shift + export test_prereq if ! test_skip "$descr" "$@" then # Announce the script to reduce confusion about the -- cgit v0.10.2-6-g49f6 From 996621eb4d4f7014a6542778420dd23b1d428896 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sun, 17 Oct 2010 02:36:59 +0800 Subject: test_terminal: catch use without TTY prerequisite It is easy to forget to declare the TTY prerequisite when writing tests on a system where it would always be satisfied (because IO::Pty is installed; see v1.7.3-rc0~33^2, 2010-08-16 for example). Automatically detect this problem so there is no need to remember. test_terminal: need to declare TTY prerequisite test_must_fail: command not found: test_terminal echo hi test_terminal returns status 127 in this case to simulate not being available. Also replace the SIMPLEPAGERTTY prerequisite on one test with "SIMPLEPAGER,TTY", since (1) the latter is supported now and (2) the prerequisite detection relies on the TTY prereq being explicitly declared. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh index 3258b8f..5e7ee9a 100644 --- a/t/lib-terminal.sh +++ b/t/lib-terminal.sh @@ -15,14 +15,23 @@ test_expect_success 'set up terminal for tests' ' if test -e have_tty then - test_terminal() { "$@"; } + test_terminal_() { "$@"; } test_set_prereq TTY elif test -e test_terminal_works then - test_terminal() { + test_terminal_() { "$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl "$@" } test_set_prereq TTY else say "# no usable terminal, so skipping some tests" fi + +test_terminal () { + if ! test_declared_prereq TTY + then + echo >&2 'test_terminal: need to declare TTY prerequisite' + return 127 + fi + test_terminal_ "$@" +} diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index 17e54d3..5641b59 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -184,11 +184,6 @@ test_expect_success 'color when writing to a file intended for a pager' ' colorful colorful.log ' -if test_have_prereq SIMPLEPAGER && test_have_prereq TTY -then - test_set_prereq SIMPLEPAGERTTY -fi - # Use this helper to make it easy for the caller of your # terminal-using function to specify whether it should fail. # If you write @@ -224,7 +219,7 @@ parse_args() { test_default_pager() { parse_args "$@" - $test_expectation SIMPLEPAGERTTY "$cmd - default pager is used by default" " + $test_expectation SIMPLEPAGER,TTY "$cmd - default pager is used by default" " unset PAGER GIT_PAGER; test_might_fail git config --unset core.pager && rm -f default_pager_used || -- cgit v0.10.2-6-g49f6 From e674c17db25750f14c1dd1d8f588eb3135b5cea9 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 16 Oct 2010 19:38:07 -0500 Subject: test_terminal: ensure redirections work reliably For terminal tests that capture output/stderr, the TTY prerequisite warning does not quite work for commands like test_terminal foo >out 2>err because the warning gets "swallowed" up by the redirection that's supposed only to be done by the subcommand. Even worse, the outcome depends on whether stdout was already a terminal (in which case test_terminal is a noop) or not (in which case test_terminal introduces a pseudo-tty in the middle of the pipeline). $ test_terminal.perl sh -c 'test -t 1 && echo >&2 YES' >out YES $ sh -c 'test -t 1 && echo >&2 YES' >out $ So: - use the test_terminal script even when running with "-v". - skip tests that require a terminal when the test_terminal script is unusable because IO::Pty is not installed. - write the "need to declare TTY prerequisite" message to fd 4, where it will be printed when running tests with -v, rather than being swallowed up by an unrelated redireciton. Noticed-by: Tay Ray Chuan Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh index 5e7ee9a..c383b57 100644 --- a/t/lib-terminal.sh +++ b/t/lib-terminal.sh @@ -1,37 +1,19 @@ #!/bin/sh test_expect_success 'set up terminal for tests' ' - if test -t 1 && test -t 2 - then - >have_tty - elif + if test_have_prereq PERL && "$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl \ sh -c "test -t 1 && test -t 2" then - >test_terminal_works + test_set_prereq TTY && + test_terminal () { + if ! test_declared_prereq TTY + then + echo >&4 "test_terminal: need to declare TTY prerequisite" + return 127 + fi + "$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl "$@" + } fi ' - -if test -e have_tty -then - test_terminal_() { "$@"; } - test_set_prereq TTY -elif test -e test_terminal_works -then - test_terminal_() { - "$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl "$@" - } - test_set_prereq TTY -else - say "# no usable terminal, so skipping some tests" -fi - -test_terminal () { - if ! test_declared_prereq TTY - then - echo >&2 'test_terminal: need to declare TTY prerequisite' - return 127 - fi - test_terminal_ "$@" -} -- cgit v0.10.2-6-g49f6 From 2d59ced10418f60914f353031215ecc6adcef204 Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sun, 17 Oct 2010 02:37:01 +0800 Subject: t5523-push-upstream: add function to ensure fresh upstream repo Signed-off-by: Tay Ray Chuan Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh index 00da707..5a18533 100755 --- a/t/t5523-push-upstream.sh +++ b/t/t5523-push-upstream.sh @@ -3,8 +3,12 @@ test_description='push with --set-upstream' . ./test-lib.sh +ensure_fresh_upstream() { + rm -rf parent && git init --bare parent +} + test_expect_success 'setup bare parent' ' - git init --bare parent && + ensure_fresh_upstream && git remote add upstream parent ' -- cgit v0.10.2-6-g49f6 From 8ac3ed27d4ca6b3c89c51e4d90e2e7be5de5ab15 Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sun, 17 Oct 2010 02:37:02 +0800 Subject: t5523-push-upstream: test progress messages Reported-by: Chase Brammer Signed-off-by: Tay Ray Chuan Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh index 5a18533..f43d760 100755 --- a/t/t5523-push-upstream.sh +++ b/t/t5523-push-upstream.sh @@ -2,6 +2,7 @@ test_description='push with --set-upstream' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-terminal.sh ensure_fresh_upstream() { rm -rf parent && git init --bare parent @@ -70,4 +71,41 @@ test_expect_success 'push -u HEAD' ' check_config headbranch upstream refs/heads/headbranch ' +test_expect_success TTY 'progress messages go to tty' ' + ensure_fresh_upstream && + + test_terminal git push -u upstream master >out 2>err && + grep "Writing objects" err +' + +test_expect_failure 'progress messages do not go to non-tty' ' + ensure_fresh_upstream && + + # skip progress messages, since stderr is non-tty + git push -u upstream master >out 2>err && + ! grep "Writing objects" err +' + +test_expect_failure 'progress messages go to non-tty (forced)' ' + ensure_fresh_upstream && + + # force progress messages to stderr, even though it is non-tty + git push -u --progress upstream master >out 2>err && + grep "Writing objects" err +' + +test_expect_success TTY 'push -q suppresses progress' ' + ensure_fresh_upstream && + + test_terminal git push -u -q upstream master >out 2>err && + ! grep "Writing objects" err +' + +test_expect_failure TTY 'push --no-progress suppresses progress' ' + ensure_fresh_upstream && + + test_terminal git push -u --no-progress upstream master >out 2>err && + ! grep "Writing objects" err +' + test_done -- cgit v0.10.2-6-g49f6 From d7c411b71d727600d2f031fec9723873ae93c1c7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 17 Oct 2010 02:37:03 +0800 Subject: push: pass --progress down to git-pack-objects When pushing via builtin transports (like file://, git://), the underlying transport helper (in this case, git-pack-objects) did not get the --progress option, even if it was passed to git push. Fix this, and update the tests to reflect this. Note that according to the git-pack-objects documentation, we can safely apply the usual --progress semantics for the transport commands like clone and fetch (and for pushing over other smart transports). Reported-by: Chase Brammer Helped-by: Jonathan Nieder Signed-off-by: Jeff King Signed-off-by: Tay Ray Chuan Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano diff --git a/builtin/send-pack.c b/builtin/send-pack.c index 481602d..efd9be6 100644 --- a/builtin/send-pack.c +++ b/builtin/send-pack.c @@ -48,6 +48,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext NULL, NULL, NULL, + NULL, }; struct child_process po; int i; @@ -59,6 +60,8 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext argv[i++] = "--delta-base-offset"; if (args->quiet) argv[i++] = "-q"; + if (args->progress) + argv[i++] = "--progress"; memset(&po, 0, sizeof(po)); po.argv = argv; po.in = -1; diff --git a/send-pack.h b/send-pack.h index 60b4ba6..05d7ab1 100644 --- a/send-pack.h +++ b/send-pack.h @@ -5,6 +5,7 @@ struct send_pack_args { unsigned verbose:1, quiet:1, porcelain:1, + progress:1, send_mirror:1, force_update:1, use_thin_pack:1, diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh index f43d760..c229fe6 100755 --- a/t/t5523-push-upstream.sh +++ b/t/t5523-push-upstream.sh @@ -78,7 +78,7 @@ test_expect_success TTY 'progress messages go to tty' ' grep "Writing objects" err ' -test_expect_failure 'progress messages do not go to non-tty' ' +test_expect_success 'progress messages do not go to non-tty' ' ensure_fresh_upstream && # skip progress messages, since stderr is non-tty @@ -86,7 +86,7 @@ test_expect_failure 'progress messages do not go to non-tty' ' ! grep "Writing objects" err ' -test_expect_failure 'progress messages go to non-tty (forced)' ' +test_expect_success 'progress messages go to non-tty (forced)' ' ensure_fresh_upstream && # force progress messages to stderr, even though it is non-tty diff --git a/transport.c b/transport.c index 4dba6f8..0078660 100644 --- a/transport.c +++ b/transport.c @@ -789,6 +789,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re args.use_thin_pack = data->options.thin; args.verbose = (transport->verbose > 0); args.quiet = (transport->verbose < 0); + args.progress = transport->progress; args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN); args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN); -- cgit v0.10.2-6-g49f6