Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> This could be convenient when tests are independent from the rest in the
> same file. Normally we would do this
>
> test_expect_success '...' '
> git init foo &&
> (
> cd foo &&
> <script>
> )
> '
>
> Now we can write a shorter version
>
> test_repo_expect_success '...' '
> <script>
> '
>
> The other function, test_subdir_expect_success, expands the script to
> "( cd <repo> && <script> )", which can be useful for grouping a series of
> tests that operate on the same repository in a subdir, e.g.
>
> test_expect_success 'create repo abc' 'test_create_repo abc'
> test_subdir_expect_success abc '...' <script>
> test_subdir_expect_success abc '...' <another-script>
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> Lately I start to add more and more tests in this style. So this
> looks like a good change to me.
Implementations of these helper functions are not all that
interesting for reviewers (except for finding unacceptable bits,
that is), I would think.
More interesting are how much cleaner the existing code would become.
I know we have tons of them that do "create a new, do this and that
in the repository".
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index e8d3c0f..45d7423 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -394,6 +394,26 @@ test_expect_success () {
> test_finish_
> }
Need a good doc here for a function that takes variable number of
parameters in a funny way.
> +test_subdir_expect_success () {
> + local subdir="$1"
This bash-ism will never be applied to my tree.
> + shift
> + case "$#" in
> + 2) test_expect_success "$1" "( cd $subdir && $2 )";;
Mental note: Two-arg form is for title and script.
By the way, unquoted subdir makes it too sloppy to live.
> + 3) test_expect_success "$1" "$2" "( cd $subdir && $3 )";;
Mental note: Three-arg form is for title, prereq and script.
> + 4) test_expect_success "$1" "$2" "$4 && ( cd $subdir && $3 )";;
What the heck is this one? That is why I said "implementations are
not interesting, the way they help existing ones more readable is".
It is totally unclear where the $4 comes from and has to be given in
a funny order to the helper (and I am sure it will make perfect
sense when it is explained).
> + *) error "bug in the test script: not 3-5 parameters to test-subdir-expect-success";;
Too deep an indent level here.
> + esac
> +}
> +
> +test_repo_expect_success () {
> + local repo=repo-$(($test_count+1))
> + case "$#" in
> + 2) test_subdir_expect_success "$repo" '' "$1" "$2" "test_create_repo $repo";;
> + 3) test_subdir_expect_success "$repo" "$1" "$2" "$3" "test_create_repo $repo";;
> + *) error "bug in the test script: not 2 or 3 parameters to test-repo-expect-success";;
> + esac
I do not like a few things in here (besides the same kind of
unacceptable stuff as the other one).
Often we need a new repository for testing a handful steps, and we
would want to be able to do this sequence:
- create a new repo
- do one thing in that repo
- do another thing in that repo
- do yet another thing in that repo
That would force tests to say "test_repo_expect_success" to do the
first thing (creating and running the first command) and then
subsequently do "test_subdir_expect_success $there" to run the
remainder.
I think we would only want to have test_expect_success_there (I am
avoiding "subdir" because the word has connotations that you do not
want in your use case. When we say "subdir" we typically do not
mean a separate repository or submodule) without the "auto creation
of a repository with unknown name" bit.
test_expect_success 'set up a new repository for testing' '
test_create_repo myrepo
'
test_expect_success_there mytest 'do one thing there' '
>empty &&
git add empty
git commit -m "add empty"
'
test_expect_success_there mytest 'do another thing there' '
git ls-files >actual &&
echo empty >expect &&
test_cmp expect actual
'
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html