diff mbox series

[v2] t7001: add failure test which triggers assertion

Message ID 29d71db280c972c91174bd0a501af66be72643af.1729462326.git.code@khaugsbakk.name (mailing list archive)
State Superseded
Headers show
Series [v2] t7001: add failure test which triggers assertion | expand

Commit Message

Kristoffer Haugsbakk Oct. 20, 2024, 10:14 p.m. UTC
From: Kristoffer Haugsbakk <code@khaugsbakk.name>

git-mv(1) gets very confused:

    git: builtin/mv.c:506: cmd_mv: Assertion `pos >= 0' failed.
    Aborted (core dumped)
    test_must_fail: died by signal 6: git mv a/a.txt a b
    fatal: Unable to create '<path>.git/index.lock': File exists.

    Another git process seems to be running in this repository, e.g.
    an editor opened by 'git commit'. Please make sure all processes
    are terminated then try again. If it still fails, a git process
    may have crashed in this repository earlier:
    remove the file manually to continue.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---

Notes (series):
    It’s been a good while.  Let’s just add this as a known breakage?

 t/t7001-mv.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)


base-commit: 34b6ce9b30747131b6e781ff718a45328aa887d0

Comments

Taylor Blau Oct. 21, 2024, 9:21 p.m. UTC | #1
On Mon, Oct 21, 2024 at 12:14:46AM +0200, kristofferhaugsbakk@fastmail.com wrote:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> git-mv(1) gets very confused:
>
>     git: builtin/mv.c:506: cmd_mv: Assertion `pos >= 0' failed.
>     Aborted (core dumped)
>     test_must_fail: died by signal 6: git mv a/a.txt a b
>     fatal: Unable to create '<path>.git/index.lock': File exists.
>
>     Another git process seems to be running in this repository, e.g.
>     an editor opened by 'git commit'. Please make sure all processes
>     are terminated then try again. If it still fails, a git process
>     may have crashed in this repository earlier:
>     remove the file manually to continue.

There was some good analysis of what the problem was in the earlier
parts of this thread. I think it is probably worth capturing some of
those here, too.

> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
>
> Notes (series):
>     It’s been a good while.  Let’s just add this as a known breakage?
>
>  t/t7001-mv.sh | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
> index 86258f9f430..739c25e2551 100755
> --- a/t/t7001-mv.sh
> +++ b/t/t7001-mv.sh
> @@ -551,4 +551,16 @@ test_expect_success 'moving nested submodules' '
>  	git status
>  '
>
> +test_expect_failure 'nonsense mv triggers assertion failure at builtin/mv.c:502' '

Do we want to be so specific about the line number that the assertion
failure occurs on? The actual coredump triggered by this test will tell
us that information. But in the meantime this line is likely to go stale
as builtin/mv.c changes over time.

> +	test_when_finished git reset --hard HEAD &&
> +	git reset --hard HEAD &&
> +	mkdir -p a &&
> +	mkdir -p b &&
> +	>a/a.txt &&
> +	git add a/a.txt &&
> +	test_must_fail git mv a/a.txt a b &&
> +	git status --porcelain >actual &&
> +	grep "^A[ ]*a/a.txt$" actual
> +'

Thanks,
Taylor
Kristoffer Haugsbakk Oct. 21, 2024, 9:25 p.m. UTC | #2
On Mon, Oct 21, 2024, at 23:21, Taylor Blau wrote:
> On Mon, Oct 21, 2024 at 12:14:46AM +0200,
> kristofferhaugsbakk@fastmail.com wrote:
>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>>
>> git-mv(1) gets very confused:
>>
>>     git: builtin/mv.c:506: cmd_mv: Assertion `pos >= 0' failed.
>>     Aborted (core dumped)
>>     test_must_fail: died by signal 6: git mv a/a.txt a b
>>     fatal: Unable to create '<path>.git/index.lock': File exists.
>>
>>     Another git process seems to be running in this repository, e.g.
>>     an editor opened by 'git commit'. Please make sure all processes
>>     are terminated then try again. If it still fails, a git process
>>     may have crashed in this repository earlier:
>>     remove the file manually to continue.
>
> There was some good analysis of what the problem was in the earlier
> parts of this thread. I think it is probably worth capturing some of
> those here, too.

I will try to incorporate Junio’s analysis into the commit message
tomorrow.  :)

>> +test_expect_failure 'nonsense mv triggers assertion failure at builtin/mv.c:502' '
>
> Do we want to be so specific about the line number that the assertion
> failure occurs on? The actual coredump triggered by this test will tell
> us that information. But in the meantime this line is likely to go stale
> as builtin/mv.c changes over time.

You’re right, it’s overly specific/volatile.

Thanks
Taylor Blau Oct. 21, 2024, 9:29 p.m. UTC | #3
On Mon, Oct 21, 2024 at 11:25:26PM +0200, Kristoffer Haugsbakk wrote:
> On Mon, Oct 21, 2024, at 23:21, Taylor Blau wrote:
> > On Mon, Oct 21, 2024 at 12:14:46AM +0200,
> > kristofferhaugsbakk@fastmail.com wrote:
> >> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
> >>
> >> git-mv(1) gets very confused:
> >>
> >>     git: builtin/mv.c:506: cmd_mv: Assertion `pos >= 0' failed.
> >>     Aborted (core dumped)
> >>     test_must_fail: died by signal 6: git mv a/a.txt a b
> >>     fatal: Unable to create '<path>.git/index.lock': File exists.
> >>
> >>     Another git process seems to be running in this repository, e.g.
> >>     an editor opened by 'git commit'. Please make sure all processes
> >>     are terminated then try again. If it still fails, a git process
> >>     may have crashed in this repository earlier:
> >>     remove the file manually to continue.
> >
> > There was some good analysis of what the problem was in the earlier
> > parts of this thread. I think it is probably worth capturing some of
> > those here, too.
>
> I will try to incorporate Junio’s analysis into the commit message
> tomorrow.  :)

Thanks, I look forward to it!

Thanks,
Taylor
diff mbox series

Patch

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 86258f9f430..739c25e2551 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -551,4 +551,16 @@  test_expect_success 'moving nested submodules' '
 	git status
 '
 
+test_expect_failure 'nonsense mv triggers assertion failure at builtin/mv.c:502' '
+	test_when_finished git reset --hard HEAD &&
+	git reset --hard HEAD &&
+	mkdir -p a &&
+	mkdir -p b &&
+	>a/a.txt &&
+	git add a/a.txt &&
+	test_must_fail git mv a/a.txt a b &&
+	git status --porcelain >actual &&
+	grep "^A[ ]*a/a.txt$" actual
+'
+
 test_done