Re: [PATCH 1/2] t4255: test am submodule with diff.submodule
On Fri, Dec 26, 2014 at 6:11 PM, Doug Kelly <dougk.ff7@gmail.com> wrote:
Show 20 quoted lines
> 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@gmail.com>
> ---
> 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) &&
Show 8 quoted lines
> +
> + 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?
Show 28 quoted lines
> + (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=$2Although 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.
Show 15 quoted lines
> + (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.
Show 20 quoted lines
> +'
> +
> +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
Doug Kelly
