On Fri, Dec 26, 2014 at 6:11 PM, Doug Kelly <dougk.ff7@xxxxxxxxx> wrote:
> git am will break when using diff.submodule=log; add some test cases
> to illustrate this breakage as simply as possible. There are
> currently two ways this can fail:
>
> * With errors ("unrecognized input"), if only change
> * Silently (no submodule change), if other files change
>
> Test for both conditions and ensure without diff.submodule this works.
>
> Signed-off-by: Doug Kelly <dougk.ff7@xxxxxxxxx>
> ---
> diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
> index 8bde7db..d9a1d79 100755
> --- a/t/t4255-am-submodule.sh
> +++ b/t/t4255-am-submodule.sh
> @@ -18,4 +18,87 @@ am_3way () {
> KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
> test_submodule_switch "am_3way"
>
> +test_expect_success 'setup diff.submodule' '
Since the tests are actually expected to fail at this point (before
you've fixed the problem), use test_expect_failure. The follow-up
patch, which fixes the problem, should flip them to
test_expect_success.
> + echo one >one &&
> + git add one &&
> + test_tick &&
> + git commit -m initial &&
Rather than performing these steps manually (here and below), perhaps
test_commit would be suitable and more succinct.
> + git rev-parse HEAD >initial &&
Other scripts in the test suite don't bother with this indirection.
Instead, they assign the variable here, then reference it in
subsequent tests (and no need to redirect to a file).
INITIAL=$(git rev-parse HEAD) &&
> +
> + git init submodule &&
> + (cd submodule &&
> + echo two >two &&
> + git add two &&
> + test_tick &&
> + git commit -m "initial submodule" &&
> + git rev-parse HEAD >../initial-submodule) &&
Style: Format the subshell like this:
(
...commands...
) &&
> + git submodule add ./submodule &&
> + test_tick &&
> + git commit -m first &&
> + git rev-parse HEAD >first &&
Is file 'first' ever used anywhere?
> + (cd submodule &&
> + echo three >three &&
> + git add three &&
> + test_tick &&
> + git commit -m "first submodule" &&
> + git rev-parse HEAD >../first-submodule) &&
> + git add submodule &&
> + test_tick &&
> + git commit -m second &&
> + git rev-parse HEAD >second &&
> +
> + (cd submodule &&
> + git mv two four &&
> + test_tick &&
> + git commit -m "second submodule" &&
> + git rev-parse HEAD >../second-submodule) &&
> + git add submodule &&
> + echo four >four &&
> + git add four &&
> + test_tick &&
> + git commit -m third &&
> + git rev-parse HEAD >third &&
> + git submodule update --init
> +'
> +
> +INITIAL=$(cat initial)
> +SECOND=$(cat second)
> +THIRD=$(cat third)
No need for this extra level of indirection. See above.
> +run_test() {
> + START_COMMIT=$1
> + EXPECT=$2
Although it's not specifically wrong here, someone adding code above
these two lines later on may not notice the broken &&-chain, so it
would be a good idea to keep the &&-chain intact.
> + (git am --abort || true) &&
> + git reset --hard $START_COMMIT &&
> + rm -f *.patch &&
> + git format-patch -1 &&
> + git reset --hard $START_COMMIT^ &&
> + git submodule update &&
> + git am *.patch &&
> + git submodule update &&
> + (cd submodule && git rev-parse HEAD >../actual) &&
> + test_cmp $EXPECT actual
> +}
> +
> +test_expect_success 'diff.submodule unset' '
> + (git config --unset diff.submodule || true) &&
> + run_test $SECOND 'first-submodule'
Note that you're already inside a single-quoted string here, so
'first-submodule' is not quite doing what you expect. Double quotes
would be more appropriate. Or, better, drop the quoting of
first-submodule altogether since it's unnecessary.
> +'
> +
> +test_expect_success 'diff.submodule unset with extra file' '
> + (git config --unset diff.submodule || true) &&
> + run_test $THIRD 'second-submodule'
> +'
> +
> +test_expect_success 'diff.submodule=log' '
> + git config diff.submodule log &&
> + run_test $SECOND 'first-submodule'
> +'
> +
> +test_expect_success 'diff.submodule=log with extra file' '
> + git config diff.submodule log &&
> + run_test $THIRD 'second-submodule'
> +'
> +
> test_done
> --
> 2.0.5
--
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