Re: [PATCHv6 16/16] Introduce portable_unset and use it to ensure proper && chaining

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Oct 4, 2010, 03:26 UTC
Message-ID: <AANLkTinU_fv7pJqw_ghM4RoF6dsiV8PFM3GgCqD-hh7L@mail.gmail.com>
In-Reply-To: <1286136014-7728-17-git-send-email-newren@gmail.com>

On Sun, Oct 3, 2010 at 20:00, Elijah Newren <newren@gmail.com> wrote:

Show 31 quoted lines

> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  t/README          |   13 +++++--------
>  t/t0001-init.sh   |   28 ++++++++++++++--------------
>  t/t7006-pager.sh  |   10 +++++-----
>  t/t7502-commit.sh |    4 ++--
>  t/test-lib.sh     |   11 +++++++++++
>  5 files changed, 37 insertions(+), 29 deletions(-)
>
> diff --git a/t/README b/t/README
> index ee4c0cf..e10c0ce 100644
> --- a/t/README
> +++ b/t/README
> @@ -259,14 +259,11 @@ Do:
>        test ...
>
>    That way all of the commands in your tests will succeed or fail. If
> -   you must ignore the return value of something (e.g., the return
> -   after unsetting a variable that was already unset is unportable) it's
> -   best to indicate so explicitly with a semicolon:
> -
> -       unset HLAGH;
> -       git merge hla &&
> -       git push gh &&
> -       test ...
> +   you must ignore the return value of something, consider using a
> +   helper function (e.g. use portable_unset instead of unset, in order
> +   to avoid unportable return value for unsetting a variable that was
> +   already unset), or prepending the command with test_might_fail or
> +   test_must_fail.

This should be:

    diff --git a/t/README b/t/README
    index a1eb7c8..dc4b9d1 100644
    --- a/t/README
    +++ b/t/README
    @@ -259,11 +259,10 @@ Do:
            test ...

        That way all of the commands in your tests will succeed or fail. If
    -   you must ignore the return value of something (e.g., the return
    -   after unsetting a variable that was already unset is unportable) it's
    -   best to indicate so explicitly with a semicolon:
    +   you must ignore the return value of something it's best to indicate
    +   so explicitly with a test_might_fail:

    -       unset HLAGH;
    +       test_might_fail some-command &&
            git merge hla &&
            git push gh &&
            test ...
    @@ -522,6 +521,10 @@ library for your script to use.
                    ...
            '

    + - portable_unset <variable>
    +
    +   .....
    +
     Prerequisites
     -------------

Or something like that. I don't know if we came to an agreement on using test_might_fail for non-builtins, but the docs on do's & dont's should mention how to do that, and portable_unset should be documented in a new section in "Test harness library".