On Mon, Apr 4, 2016 at 1:36 PM, Mehul Jain <mehul.jain2029@xxxxxxxxx> wrote:
> On Mon, Apr 4, 2016 at 10:22 PM, Matthieu Moy
> <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
>> I think it would be much simpler to drop the loop, and write instead
>> something like (untested):
>
> I tested it (with few minor changes), and worked fine.
>
> test_autostash () {
> OLDIFS=$IFS
> IFS='='
> set -- $*
> IFS=$OLDIFS
> expect=$1
> cmd=$2
> config_variable=$3
> value=$4
> test_expect_success "$cmd, $config_variable=$value" '
> if [ "$value" = "" ]; then
> test_unconfig $config_variable
> else
> test_config $config_variable $value
> fi &&
>
> git reset --hard before-rebase &&
> echo dirty >new_file &&
> git add new_file &&
>
> if [ $expect = "ok" ]; then
> git pull $cmd . copy &&
> test_cmp_rev HEAD^ copy &&
> test "$(cat new_file)" = dirty &&
> test "$(cat file)" = "modified again"
> else
> test_must_fail git pull $cmd . copy 2>err &&
> test_i18ngrep "uncommitted changes." err
> fi
> '
> }
>
> test_autostash ok '--rebase' rebase.autostash=true
> test_autostash ok '--rebase --autostash' rebase.autostash=true
> test_autostash ok '--rebase --autostash' rebase.autostash=false
> test_autostash ok '--rebase --autostash' rebase.autostash=
> test_autostash err '--rebase --no-autostash' rebase.autostash=true
> test_autostash err '--rebase --no-autostash' rebase.autostash=false
> test_autostash err '--rebase --no-autostash' rebase.autostash=
> test_autostash ok '--autostash' pull.rebase=true
> test_autostash err '--no-autostash' pull.rebase=true
>
> Perhaps this looks better than the one with the loop. Even better than
> the implementation in v2[1].
>
> I think it would be wise to go with the above script for v3 (as I will
> be doing a re-roll of the series[1]).
This new function is sufficiently complex that it increases cognitive
load enough for me to question if it is really a win for such a small
number of tests. The individual tests, as implemented in the current
round, are quite easy to understand, and don't place any significant
cognitive burden on the reader.
Although I'm the one who brought up the idea of "automating" these
tests, I'm not convinced that it's an improvement in this case, but I
don't feel so strongly that I'd forbid it. So, choose the approach
which seems best to you while weighing comprehension load for people
new to these tests, as well as maintainability costs.
--
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