Re: [PATCH v5 2/7] reset: add a few tests for "git reset --merge"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Christian Couder <chriscool@xxxxxxxxxxxxx> writes:

> +test_description='Tests for "git reset --merge"'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'creating initial files' '
> +     echo "line 1" >> file1 &&
> +     echo "line 2" >> file1 &&
> +     echo "line 3" >> file1 &&
> +     cp file1 file2 &&
> +     git add file1 file2 &&
> +     test_tick &&
> +     git commit -m "Initial commit"
> +'

5-char indent is somewhat unusual; please fix it.  Also you may want to
tag the initial one and other key commits for later use.

> +test_expect_success 'reset --merge is ok with changes in file it does not touch' '
> +     echo "line 4" >> file1 &&
> +     echo "line 4" >> file2 &&
> +     test_tick &&
> +     git commit -m "add line 4" file1 &&

IOW, move the above four lines to the end of the first "setup", and tag as
necessary like this:

    test_expect_success setup '
        for i in 1 2 3; do echo line $i; done >file1 &&
        cat file1 >file2 &&
        git add file1 file2 &&
        test_tick &&
        git commit -m "Initial commit" &&
        git tag initial &&
        echo line 4 >>file1 &&
        cat file1 >file2 &&
        test_tick &&
        git commit -m "add line 4 to file1" file1 &&
        git tag second
    '

And the rest should become its own test, starting from a known state.

> +     git reset --merge HEAD^ &&
> +     ! grep 4 file1 &&
> +     grep 4 file2 &&

Switching from the second commit to the initial one should succeed because
the only local change is in file2 that is unrelated.  This should also check
if HEAD points at the "initial" commit after "reset", and the index
matches the HEAD commit.

> +     git reset --merge HEAD@{1} &&
> +     grep 4 file1 &&
> +     grep 4 file2
> +'

And switching back to the second commit should recover.  This should be a
separate test, and should make sure it begins in a known state even if the
"reset --merge HEAD^" in the previous test failed.  This should also
check:

	test $(git rev-parse HEAD) = $(git rev-parse second) &&
	test -z "$(git diff --cached)"

If any of these two "reset --merge" is broken, the next test will start at
an unknown commit, hence you should add

	git reset --hard second &&

at the beginning of the next test.  You may also need to copy file1 to
file2 to recreate the local change as well.

The idea is not to assume that tests will succeed and 'recovery' step at
the end will run; instead, make sure each test starts itself in a known
state.  The way you wrote "reset --hard" at the end of some tests for
recovery is fragile.  There is no guarantee that each test run to the end,
executing these recovery parts.  Instead, have a corresponding set-up at
the beginning of each test as necessary.

> +test_expect_success 'reset --merge discards changes added to index (1)' '
> +     echo "line 5" >> file1 &&
> +     git add file1 &&

So at this point, file2 has an extra "line 4", and file1 has a change to
be committed.  And we reset to the initial commit.

> +     git reset --merge HEAD^ &&
> +     ! grep 4 file1 &&
> +     ! grep 5 file1 &&
> +     grep 4 file2 &&

And that will make file1 match that of initial, while keeping the change
to file2 intact.  The same comment about missing checks applies.  The
remainder of this test should be a separate test.

> +     echo "line 5" >> file2 &&
> +     git add file2 &&
> +     git reset --merge HEAD@{1} &&
> +     ! grep 4 file2 &&
> +     ! grep 5 file1 &&
> +     grep 4 file1
> +'

Starting at 'initial' but with two lines added to file2 and the index
updated to it, we reset to 'second' (spell these out with tags, instead of
relying on reflog, so that you do not assume all the previous tests have
passed).  Both files should match those of 'second'.  The same comment
about missing checks applies.

> +test_expect_success 'reset --merge discards changes added to index (2)' '
> +     echo "line 4" >> file2 &&
> +     git add file2 &&
> +     git reset --merge HEAD^ &&
> +     ! grep 4 file2 &&
> +     git reset --merge HEAD@{1} &&
> +     ! grep 4 file2 &&
> +     grep 4 file1
> +'

The same comment about the missing 'make sure it begins in a known state'
applies, but I don't see the point of this (2).  Is there any fundamental
difference of the set-up this one tests, compared to the earlier test?

> +test_expect_success 'reset --merge fails with changes in file it touches' '
> +     echo "line 5" >> file1 &&
> +     test_tick &&
> +     git commit -m "add line 5" file1 &&
> +     sed -e "s/line 1/changed line 1/" <file1 >file3 &&
> +     mv file3 file1 &&
> +     test_must_fail git reset --merge HEAD^ 2>err.log &&
> +     grep file1 err.log | grep "not uptodate" &&

Hmm, this is something we had a patch for to give different messages from
plumbing and Porcelain.

> +     git reset --hard HEAD^

Lose this 'recover at the end'.  There is no guarantee the each test run
to the end.   Instead, have a corresponding set-up at the beginning of the
next one.

I'd re-review the rest after this is rerolled.
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]