Subject: Re: [PATCH 2/9] t7006: test pager configuration for several git commands

From: Johannes Sixt <[email-masked]>
Date: Tue, 13 Apr 2010 08:34:38 +0200

Am 4/13/2010 4:17, schrieb Jonathan Nieder:
> +test_pager_choice() {
> +...
> +	test_expect_success SIMPLEPAGER "$cmd - default pager is used by default" "
> +		cat > $less <<-\EOF &&
> +		#!$SHELL_PATH

This shell script is simple enough that you can use '#!/bin/sh' and avoid
$ expansion where one must ask whether it needs quoting or not.

> +		wc > default_pager_used
> +		EOF
> +...
> +	"
> +...
> +	test_expect_success TTY "$cmd - PAGER overrides default pager" "
> +...
> +	$test_expect_expected TTY "$cmd - core.pager in subdir" "
> +...
> +	test_expect_success TTY "$cmd - GIT_PAGER overrides core.pager" "
> +...
>  
> +test_pager_choice 'git log'
> +test_pager_choice 'git -p log'
> +test_pager_choice test_must_fail 'git -p'
> +test_pager_choice test_must_fail 'git -p nonsense'

The intents of the test cases would be much easier to follow and to review
if this were written as, for example:

test_default_pager        'git log'
test_PAGER_overrides      'git log'
test_core_pager_overrides 'git log'
test_core_pager_subdir    'git log'
test_GIT_PAGER_overrides  'git log'
test_default_pager        'git -p log'
test_PAGER_overrides      'git -p log'
test_core_pager_overrides 'git -p log'
test_core_pager_subdir    'git -p log' fails
test_GIT_PAGER_overrides  'git -p log'
test_default_pager        'test_must_fail git -p'
test_PAGER_overrides      'test_must_fail git -p'
test_core_pager_overrides 'test_must_fail git -p'
test_core_pager_subdir    'test_must_fail git -p' fails
test_GIT_PAGER_overrides  'test_must_fail git -p'
test_default_pager        'test_must_fail git -p nonsense'
test_PAGER_overrides      'test_must_fail git -p nonsense'
test_core_pager_overrides 'test_must_fail git -p nonsense'
test_core_pager_subdir    'test_must_fail git -p nonsense' fails
test_GIT_PAGER_overrides  'test_must_fail git -p nonsense'

the point being that it is less factorized (not the exact syntax).

-- Hannes