[PATCH 0/8] Improvements for t/README

[PATCH 0/8] Improvements for t/README

[Ævar Arnfjörð Bjarmason]

Here are the the t/README improvements I promised Junio in
<AANLkTilYoGtTIr9lIQD4sx4PRjlWYjIwSDMquREzv6eq@mail.gmail.com>, and
more. I took the opportunity while I was at it to document some
non-TAP things in the test suite like the prereq functions and
test_external_*.

Ævar Arnfjörð Bjarmason (8):
  t/README: Tests are all +x, ./test, not sh ./test
  t/README: The trash is in 't/trash directory.$name'
  t/README: Typo: paralell -> parallel
  t/README: Document the prereq functions, and 3-arg test_*
  t/README: Document test_external*
  t/README: Document test_expect_code
  t/README: Add a section about skipping tests
  t/README: Document the dangers of printing "ok" / "not ok"

 t/README |  100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 92 insertions(+), 8 deletions(-)

[PATCH 1/8] t/README: Tests are all +x, ./test, not sh ./test

[Ævar Arnfjörð Bjarmason]

They all run with a #!/bin/sh shebang, but it's probably better to use
the ./ form anyway.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/README |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/README b/t/README
index aa4ed28..b378ab1 100644
--- a/t/README
+++ b/t/README
@@ -52,7 +52,7 @@ prove and other harnesses come with a lot of useful options. The
 
 You can also run each test individually from command line, like this:
 
-    $ sh ./t3010-ls-files-killed-modified.sh
+    $ ./t3010-ls-files-killed-modified.sh
     ok 1 - git update-index --add to add various paths.
     ok 2 - git ls-files -k to show killed files.
     ok 3 - validate git ls-files -k output.
-- 
1.7.1.251.g92a7

Re: [PATCH 1/8] t/README: Tests are all +x, ./test, not sh ./test

[Jeff King]

On Thu, Jul 01, 2010 at 03:09:57PM +0000, Ævar Arnfjörð Bjarmason wrote:

> They all run with a #!/bin/sh shebang, but it's probably better to use
> the ./ form anyway.

If we want to be nitpicky, they are actually run by "make test" with
$SHELL_PATH. We don't re-write the shebang lines in the test scripts (as
we do with the actual git-* scripts), so running "./t-whatever" won't
work if you have a broken shell (e.g., Solaris). You have to do "bash
./t-whatever".

-Peff

[PATCH 2/8] t/README: The trash is in 't/trash directory.$name'

[Ævar Arnfjörð Bjarmason]

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/README |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/README b/t/README
index b378ab1..fd61ec7 100644
--- a/t/README
+++ b/t/README
@@ -221,9 +221,10 @@ This test harness library does the following things:
  - If the script is invoked with command line argument --help
    (or -h), it shows the test_description and exits.
 
- - Creates an empty test directory with an empty .git/objects
-   database and chdir(2) into it.  This directory is 't/trash directory'
-   if you must know, but I do not think you care.
+ - Creates an empty test directory with an empty .git/objects database
+   and chdir(2) into it.  This directory is 't/trash
+   directory.$test_name_without_dotsh' if you must know, but I do not
+   think you care.
 
  - Defines standard test helper functions for your scripts to
    use.  These functions are designed to make all scripts behave
-- 
1.7.1.251.g92a7

Re: [PATCH 2/8] t/README: The trash is in 't/trash directory.$name'

[Jeff King]

On Thu, Jul 01, 2010 at 03:09:58PM +0000, Ævar Arnfjörð Bjarmason wrote:

> diff --git a/t/README b/t/README
> index b378ab1..fd61ec7 100644
> --- a/t/README
> +++ b/t/README
> @@ -221,9 +221,10 @@ This test harness library does the following things:
>   - If the script is invoked with command line argument --help
>     (or -h), it shows the test_description and exits.
>  
> - - Creates an empty test directory with an empty .git/objects
> -   database and chdir(2) into it.  This directory is 't/trash directory'
> -   if you must know, but I do not think you care.
> + - Creates an empty test directory with an empty .git/objects database
> +   and chdir(2) into it.  This directory is 't/trash
> +   directory.$test_name_without_dotsh' if you must know, but I do not
> +   think you care.

You might want to mention the impact of "--root" here. I.e., it is
actually "$root${root:+/}trash directory.$test_name_without_dotsh", and
it is in t/ only if $root is not an absolute path.

-Peff

[PATCH 3/8] t/README: Typo: paralell -> parallel

[Ævar Arnfjörð Bjarmason]

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/README |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/README b/t/README
index fd61ec7..0998bfb 100644
--- a/t/README
+++ b/t/README
@@ -33,7 +33,7 @@ the tests.
     ok 3 - plain bare
 
 Since the tests all output TAP (see http://testanything.org) they can
-be run with any TAP harness. Here's an example of paralell testing
+be run with any TAP harness. Here's an example of parallel testing
 powered by a recent version of prove(1):
 
     $ prove --timer --jobs 15 ./t[0-9]*.sh
-- 
1.7.1.251.g92a7

[PATCH 4/8] t/README: Document the prereq functions, and 3-arg test_*

[Ævar Arnfjörð Bjarmason]

There was no documentation for the test_set_prereq and
test_have_prereq functions, or the three-arg form of
test_expect_success and test_expect_failure.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/README |   37 ++++++++++++++++++++++++++++++++++---
 1 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/t/README b/t/README
index 0998bfb..67f8f30 100644
--- a/t/README
+++ b/t/README
@@ -246,9 +246,9 @@ Test harness library
 There are a handful helper functions defined in the test harness
 library for your script to use.
 
- - test_expect_success <message> <script>
+ - test_expect_success [<prereq>] <message> <script>
 
-   This takes two strings as parameter, and evaluates the
+   Usually takes two strings as parameter, and evaluates the
    <script>.  If it yields success, test is considered
    successful.  <message> should state what it is testing.
 
@@ -257,8 +257,15 @@ library for your script to use.
 	test_expect_success \
 	    'git-write-tree should be able to write an empty tree.' \
 	    'tree=$(git-write-tree)'
+        
+   If you supply three parameters the first will be taken to be a
+   prerequisite, see the test_set_prereq and test_have_prereq
+   documentation below:
 
- - test_expect_failure <message> <script>
+	test_expect_success TTY 'git --paginate rev-list uses a pager' \
+	    ' ... '
+
+ - test_expect_failure [<prereq>] <message> <script>
 
    This is NOT the opposite of test_expect_success, but is used
    to mark a test that demonstrates a known breakage.  Unlike
@@ -266,6 +273,9 @@ library for your script to use.
    success and "FAIL" on failure, this will say "FIXED" on
    success and "still broken" on failure.  Failures from these
    tests won't cause -i (immediate) to stop.
+   
+   Like test_expect_success this function can optionally use a three
+   argument invocation with a prerequisite as the first argument.
 
  - test_debug <script>
 
@@ -299,6 +309,27 @@ library for your script to use.
    Merges the given rev using the given message.  Like test_commit,
    creates a tag and calls test_tick before committing.
 
+ - test_set_prereq SOME_PREREQ
+ 
+   Set a test prerequisite to be used later with test_have_prereq. The
+   test-lib will set some prerequisites for you, e.g. PERL and PYTHON
+   which are derived from ./GIT-BUILD-OPTIONS (grep test_set_prereq
+   test-lib.sh for more). Others you can set yourself and use later
+   with either test_have_prereq directly, or the three argument
+   invocation of test_expect_success and test_expect_failure.
+   
+ - test_have_prereq SOME PREREQ
+
+   Check if we have a prerequisite previously set with
+   test_set_prereq. The most common use of this directly is to skip
+   all the tests if we don't have some essential prerequisite:
+ 
+	if ! test_have_prereq PERL
+	then
+	    skip_all='skipping perl interface tests, perl not available'
+	    test_done
+	fi
+
 Tips for Writing Tests
 ----------------------
 
-- 
1.7.1.251.g92a7

Re: [PATCH 4/8] t/README: Document the prereq functions, and 3-arg test_*

[Junio C Hamano]

Thanks.  We should have done this one long time ago.

[PATCH 5/8] t/README: Document test_external*

[Ævar Arnfjörð Bjarmason]

There was do documentation for the test_external_without_stderr and
test_external functions.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/README |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/t/README b/t/README
index 67f8f30..43d6d4d 100644
--- a/t/README
+++ b/t/README
@@ -329,6 +329,33 @@ library for your script to use.
 	    skip_all='skipping perl interface tests, perl not available'
 	    test_done
 	fi
+    
+ - test_external [<prereq>] <message> <external> <script>
+ 
+   Execute a <script> with an <external> interpreter (like perl). This
+   was added for tests like t9700-perl-git.sh which do most of their
+   work in an external test script.
+   
+	test_external_without_stderr \
+	    'Perl API' \
+	    "$PERL_PATH" "$TEST_DIRECTORY"/t9700/test.pl
+
+   If the test is outputting its own TAP you should set the
+   test_external_has_tap variable somewhere before calling the first
+   test_external* function. See t9700-perl-git.sh for an example.
+
+	# The external test will outputs its own plan
+	test_external_has_tap=1
+    
+ - test_external_without_stderr [<prereq>] <message> <external> <script>
+ 
+   Like test_external but fail if there's any output on stderr,
+   instead of checking the exit code.
+ 
+	test_external_without_stderr \
+	    'Perl API' \
+	    "$PERL_PATH" "$TEST_DIRECTORY"/t9700/test.pl
+
 
 Tips for Writing Tests
 ----------------------
-- 
1.7.1.251.g92a7

Re: [PATCH 5/8] t/README: Document test_external*

[Junio C Hamano]

Thanks.  We should have done this one long time ago, too.

[PATCH 6/8] t/README: Document test_expect_code

[Ævar Arnfjörð Bjarmason]

test_expect_code (which was introduced in d3bfdb75) never had any
documentation.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/README |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/t/README b/t/README
index 43d6d4d..6232567 100644
--- a/t/README
+++ b/t/README
@@ -276,6 +276,13 @@ library for your script to use.
    
    Like test_expect_success this function can optionally use a three
    argument invocation with a prerequisite as the first argument.
+   
+ - test_expect_code [<prereq>] <code> <message> <script>
+ 
+   Analogous to test_expect_success, but pass the test if it exits
+   with a given exit <code>
+ 
+ test_expect_code 1 'Merge with d/f conflicts' 'git merge "merge msg" B master'
 
  - test_debug <script>
 
-- 
1.7.1.251.g92a7

[PATCH 7/8] t/README: Add a section about skipping tests

[Ævar Arnfjörð Bjarmason]

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/README |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/t/README b/t/README
index 6232567..42a4c25 100644
--- a/t/README
+++ b/t/README
@@ -231,6 +231,18 @@ This test harness library does the following things:
    consistently when command line arguments --verbose (or -v),
    --debug (or -d), and --immediate (or -i) is given.
 
+Skipping tests
+--------------
+
+If you need to skip all the remaining tests you should set skip_all
+and immediately call test_done. The string you give to skip_all will
+be used as an explanation for why the test was skipped. for instance:
+
+	if ! test_have_prereq PERL
+	then
+	    skip_all='skipping perl interface tests, perl not available'
+	    test_done
+	fi
 
 End with test_done
 ------------------
-- 
1.7.1.251.g92a7

[PATCH 8/8] t/README: Document the dangers of printing "ok" / "not ok"

[Ævar Arnfjörð Bjarmason]

A TAP harness will get confused if you print "ok" or "not ok" at the
beginning of a line. This had to be fixed in
335f87871fe5aa6b3fd55b2b4e80f16fe9681483 when TAP support was
introduced.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/README |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/t/README b/t/README
index 42a4c25..29f795e 100644
--- a/t/README
+++ b/t/README
@@ -50,6 +50,12 @@ prove and other harnesses come with a lot of useful options. The
     # Repeat until no more failures
     $ prove -j 15 --state=failed,save ./t[0-9]*.sh
 
+The TAP support is completely incidental. A TAP harness is just a
+program that interprets the "ok"/"not ok" (and some other strings) in
+a special way. The only limitation this imposes is that you shouldn't
+echo "ok" or "not ok" by yourself at the beginning of a line, that'll
+confuse the TAP harness.
+
 You can also run each test individually from command line, like this:
 
     $ ./t3010-ls-files-killed-modified.sh
-- 
1.7.1.251.g92a7

Re: [PATCH 8/8] t/README: Document the dangers of printing "ok" / "not ok"

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> index 42a4c25..29f795e 100644
> --- a/t/README
> +++ b/t/README
> @@ -50,6 +50,12 @@ prove and other harnesses come with a lot of useful options. The
>      # Repeat until no more failures
>      $ prove -j 15 --state=failed,save ./t[0-9]*.sh
>  
> +The TAP support is completely incidental. A TAP harness is just a
> +program that interprets the "ok"/"not ok" (and some other strings) in
> +a special way. The only limitation this imposes is that you shouldn't
> +echo "ok" or "not ok" by yourself at the beginning of a line, that'll
> +confuse the TAP harness.
> +

I think the above is not suitable for the target audience of the section
("Running Tests"); they are not _writing_ tests.

This information (or an enhanced version of it) should come between
"Source 'test-lib.sh'" that talks about what the harness gives, and
"Skipping tests".  Perhaps name that to "Test your stuff".  An example...

    Test your stuff
    ---------------

    After sourcing test-lib.sh, you test the parts of the system using the
    test harness.  Your tests will be done in the <script> part of
    test_expect_success helper function, and the exit status of the last
    command in that <script> signals the success/failure of a test.

    You are not supposed to "exit" from the <script> part; the harness
    will catch this as a programming error of the test.  Use test_done
    instead, if you need to stop the tests early (see "Skipping tests"
    below).

    Inside <script> part, the standard output and standard error streams
    are discarded, and the test harness only reports "ok" or "not ok" to
    the end user running the tests.  Under --verbose, they are shown to
    help debugging the tests.

    The output from the test suite is designed to be TAP compliant (see
    http://..... to learn more about TAP), to allow programs by other
    people to parse our output.  This further places restriction on what
    your tests can say to the standard output and standard error.  The TAP
    readers expect a line that begins with either "ok" and "not ok" to
    signal a test passed or failed (and our harness already produces such
    lines), so your script shouldn't emit such lines to their output.

I suspect the last paragraph needs a bit more work.  For example, don't
you have some restriction not to say things like "1..43"???