On Mon, Aug 15, 2016 at 2:53 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> Tests consisting of one line each can be consolidated to have fewer tests
> to run as well as fewer lines of code.
>
> When having just a few git commands, do not create a new shell but
> use the -C flag in Git to execute in the correct directory.
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
Looks good. The resulting test file is easier to read, and you created
a common function to perform checking if we used alternates instead of
duplicating that part multiple times.
> t/t7408-submodule-reference.sh | 48 ++++++++++++++++++++++--------------------
> 1 file changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
> index b84c6748..dff47af 100755
> --- a/t/t7408-submodule-reference.sh
> +++ b/t/t7408-submodule-reference.sh
> @@ -8,6 +8,15 @@ test_description='test clone --reference'
>
> base_dir=$(pwd)
>
> +test_alternate_is_used () {
> + alternates_file="$1" &&
> + working_dir="$2" &&
> + test_line_count = 1 "$alternates_file" &&
> + echo "0 objects, 0 kilobytes" >expect &&
> + git -C "$working_dir" count-objects >actual &&
> + test_cmp expect actual
> +}
> +
This change wasn't mentioned in the description. You updated the tests
to use a stronger check for when alternates is in use. This is good. I
might have mentioned it in the description but it's not worth a
re-roll.
> -test_expect_success 'that reference gets used with update' '
> - cd super-clone/sub &&
> - echo "0 objects, 0 kilobytes" >expected &&
> - git count-objects >current &&
> - diff expected current
The stronger variant was used once here, but you use it in several
locations. It's good to have extracted this as the new tests are more
readable.
Thanks,
Jake
--
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