* [PATCH v2 1/3] Add tests for describe with --work-tree
@ 2019-01-26 20:49 Sebastian Staudt
2019-01-26 20:49 ` [PATCH v2 2/3] Setup working tree in describe Sebastian Staudt
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Sebastian Staudt @ 2019-01-26 20:49 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Sebastian Staudt
The dirty ones are already passing, but just because describe is comparing
with the wrong working tree.
Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
---
t/t6120-describe.sh | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index d639d94696..9a6bd1541f 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -28,6 +28,24 @@ check_describe () {
'
}
+check_describe_worktree () {
+ cd "$TEST_DIRECTORY"
+ expect="$1"
+ shift
+ R=$(git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe "$@" 2>err.actual)
+ S=$?
+ cat err.actual >&3
+ test_expect_success "describe with --work-tree $*" '
+ test $S = 0 &&
+ case "$R" in
+ $expect) echo happy ;;
+ *) echo "Oops - $R is not $expect";
+ false ;;
+ esac
+ '
+ cd "$TRASH_DIRECTORY"
+}
+
test_expect_success setup '
test_tick &&
@@ -145,14 +163,20 @@ check_describe A-* HEAD
check_describe "A-*[0-9a-f]" --dirty
+check_describe_worktree "A-*[0-9a-f]" --dirty
+
test_expect_success 'set-up dirty work tree' '
echo >>file
'
check_describe "A-*[0-9a-f]-dirty" --dirty
+check_describe_worktree "A-*[0-9a-f]-dirty" --dirty
+
check_describe "A-*[0-9a-f].mod" --dirty=.mod
+check_describe_worktree "A-*[0-9a-f].mod" --dirty=.mod
+
test_expect_success 'describe --dirty HEAD' '
test_must_fail git describe --dirty HEAD
'
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/3] Setup working tree in describe
2019-01-26 20:49 [PATCH v2 1/3] Add tests for describe with --work-tree Sebastian Staudt
@ 2019-01-26 20:49 ` Sebastian Staudt
2019-01-27 0:21 ` Duy Nguyen
2019-01-26 20:49 ` [PATCH v2 3/3] Add test for describe with a bare repository Sebastian Staudt
2019-01-27 0:07 ` [PATCH v2 1/3] Add tests for describe with --work-tree Duy Nguyen
2 siblings, 1 reply; 11+ messages in thread
From: Sebastian Staudt @ 2019-01-26 20:49 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Sebastian Staudt
This ensures the given working tree is used for --dirty.
The implementation of --broken uses diff-index which calls
setup_work_tree() itself.
Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
---
builtin/describe.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/describe.c b/builtin/describe.c
index cc118448ee..b5b7abdc8f 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -629,6 +629,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
struct argv_array args = ARGV_ARRAY_INIT;
int fd, result;
+ setup_work_tree();
read_cache();
refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED,
NULL, NULL, NULL);
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/3] Add test for describe with a bare repository
2019-01-26 20:49 [PATCH v2 1/3] Add tests for describe with --work-tree Sebastian Staudt
2019-01-26 20:49 ` [PATCH v2 2/3] Setup working tree in describe Sebastian Staudt
@ 2019-01-26 20:49 ` Sebastian Staudt
2019-01-27 0:25 ` Duy Nguyen
2019-01-27 0:07 ` [PATCH v2 1/3] Add tests for describe with --work-tree Duy Nguyen
2 siblings, 1 reply; 11+ messages in thread
From: Sebastian Staudt @ 2019-01-26 20:49 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Sebastian Staudt
This ensures that nothing breaks the basic functionality of describe for
bare repositories. Please note that --broken and --dirty need a working
tree.
Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
---
t/t6120-describe.sh | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 9a6bd1541f..ddd8cc307d 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -409,4 +409,11 @@ test_expect_success 'describe complains about missing object' '
test_must_fail git describe $ZERO_OID
'
+test_expect_success 'describe works from outside repo using --git-dir' "
+ BARE_CLONE=$(mktemp -d) &&
+ git clone --bare '$TRASH_DIRECTORY' \$BARE_CLONE >/Users/koraktor/open-source/others/git/t/out &&
+ echo $PWD >/Users/koraktor/open-source/others/git/t/out &&
+ git --git-dir \$BARE_CLONE describe 2>&1 >/Users/koraktor/open-source/others/git/t/out
+"
+
test_done
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] Add tests for describe with --work-tree
2019-01-26 20:49 [PATCH v2 1/3] Add tests for describe with --work-tree Sebastian Staudt
2019-01-26 20:49 ` [PATCH v2 2/3] Setup working tree in describe Sebastian Staudt
2019-01-26 20:49 ` [PATCH v2 3/3] Add test for describe with a bare repository Sebastian Staudt
@ 2019-01-27 0:07 ` Duy Nguyen
2019-01-27 7:13 ` Sebastian Staudt
2 siblings, 1 reply; 11+ messages in thread
From: Duy Nguyen @ 2019-01-27 0:07 UTC (permalink / raw)
To: Sebastian Staudt; +Cc: Git Mailing List, Junio C Hamano, Jeff King
On Sun, Jan 27, 2019 at 3:51 AM Sebastian Staudt <koraktor@gmail.com> wrote:
>
> The dirty ones are already passing, but just because describe is comparing
> with the wrong working tree.
>
> Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> ---
> t/t6120-describe.sh | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> index d639d94696..9a6bd1541f 100755
> --- a/t/t6120-describe.sh
> +++ b/t/t6120-describe.sh
> @@ -28,6 +28,24 @@ check_describe () {
> '
> }
>
> +check_describe_worktree () {
> + cd "$TEST_DIRECTORY"
Strange alignment. We normally do it in a subshell...
> + expect="$1"
> + shift
> + R=$(git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe "$@" 2>err.actual)
These commands should be executed inside test_expect_success, not
outside. And you need to chain commands with && to make sure if
something breaks, then the whole test will fai.
If it's too ugly to generate test_expect_success with a shell
function, then just write a shell function that "describe" and compare
(i.e. the test body). Then you can write something like this later
test_expect_sucesss 'describe with --worktree foo' '
check_describe_worktree foo
'
and check_describe_worktree can now do
( cd "$TEST_DIRECTORY" && .... )
> + S=$?
> + cat err.actual >&3
> + test_expect_success "describe with --work-tree $*" '
> + test $S = 0 &&
> + case "$R" in
> + $expect) echo happy ;;
> + *) echo "Oops - $R is not $expect";
> + false ;;
> + esac
> + '
> + cd "$TRASH_DIRECTORY"
> +}
> +
> test_expect_success setup '
>
> test_tick &&
> @@ -145,14 +163,20 @@ check_describe A-* HEAD
>
> check_describe "A-*[0-9a-f]" --dirty
>
> +check_describe_worktree "A-*[0-9a-f]" --dirty
> +
> test_expect_success 'set-up dirty work tree' '
> echo >>file
> '
>
> check_describe "A-*[0-9a-f]-dirty" --dirty
>
> +check_describe_worktree "A-*[0-9a-f]-dirty" --dirty
> +
> check_describe "A-*[0-9a-f].mod" --dirty=.mod
>
> +check_describe_worktree "A-*[0-9a-f].mod" --dirty=.mod
> +
> test_expect_success 'describe --dirty HEAD' '
> test_must_fail git describe --dirty HEAD
> '
> --
> 2.20.1
>
--
Duy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/3] Setup working tree in describe
2019-01-26 20:49 ` [PATCH v2 2/3] Setup working tree in describe Sebastian Staudt
@ 2019-01-27 0:21 ` Duy Nguyen
2019-01-27 7:19 ` Sebastian Staudt
0 siblings, 1 reply; 11+ messages in thread
From: Duy Nguyen @ 2019-01-27 0:21 UTC (permalink / raw)
To: Sebastian Staudt; +Cc: Git Mailing List, Junio C Hamano, Jeff King
On Sun, Jan 27, 2019 at 3:51 AM Sebastian Staudt <koraktor@gmail.com> wrote:
>
> This ensures the given working tree is used for --dirty.
>
> The implementation of --broken uses diff-index which calls
> setup_work_tree() itself.
It would be nice to have a test case covering --broken even if no fix
is needed (so that somebody else will not accidentally break it
later). I did a quick test and thought it was broken, but it turns out
I tested it wrong :P
> Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> ---
> builtin/describe.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/builtin/describe.c b/builtin/describe.c
> index cc118448ee..b5b7abdc8f 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -629,6 +629,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
> struct argv_array args = ARGV_ARRAY_INIT;
> int fd, result;
>
> + setup_work_tree();
> read_cache();
> refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED,
> NULL, NULL, NULL);
> --
> 2.20.1
>
--
Duy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/3] Add test for describe with a bare repository
2019-01-26 20:49 ` [PATCH v2 3/3] Add test for describe with a bare repository Sebastian Staudt
@ 2019-01-27 0:25 ` Duy Nguyen
2019-01-27 6:54 ` Sebastian Staudt
0 siblings, 1 reply; 11+ messages in thread
From: Duy Nguyen @ 2019-01-27 0:25 UTC (permalink / raw)
To: Sebastian Staudt; +Cc: Git Mailing List, Junio C Hamano, Jeff King
On Sun, Jan 27, 2019 at 3:51 AM Sebastian Staudt <koraktor@gmail.com> wrote:
>
> This ensures that nothing breaks the basic functionality of describe for
> bare repositories. Please note that --broken and --dirty need a working
> tree.
>
> Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> ---
> t/t6120-describe.sh | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> index 9a6bd1541f..ddd8cc307d 100755
> --- a/t/t6120-describe.sh
> +++ b/t/t6120-describe.sh
> @@ -409,4 +409,11 @@ test_expect_success 'describe complains about missing object' '
> test_must_fail git describe $ZERO_OID
> '
>
> +test_expect_success 'describe works from outside repo using --git-dir' "
> + BARE_CLONE=$(mktemp -d) &&
No, keep everything in $TRASH_DIRECTORY so it will be automatically cleaned.
> + git clone --bare '$TRASH_DIRECTORY' \$BARE_CLONE >/Users/koraktor/open-source/others/git/t/out &&
Ehh.. I'm pretty sure I don't have /Users/koraktor on my system :)
This looks like just debug code, I think you can drop ">.." part for
all commands.
> + echo $PWD >/Users/koraktor/open-source/others/git/t/out &&
> + git --git-dir \$BARE_CLONE describe 2>&1 >/Users/koraktor/open-source/others/git/t/out
> +"
> +
> test_done
> --
> 2.20.1
>
--
Duy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/3] Add test for describe with a bare repository
2019-01-27 0:25 ` Duy Nguyen
@ 2019-01-27 6:54 ` Sebastian Staudt
0 siblings, 0 replies; 11+ messages in thread
From: Sebastian Staudt @ 2019-01-27 6:54 UTC (permalink / raw)
To: Duy Nguyen, Git Mailing List; +Cc: Junio C Hamano, Jeff King
Am So., 27. Jan. 2019 um 01:25 Uhr schrieb Duy Nguyen <pclouds@gmail.com>:
>
> On Sun, Jan 27, 2019 at 3:51 AM Sebastian Staudt <koraktor@gmail.com> wrote:
> >
> > This ensures that nothing breaks the basic functionality of describe for
> > bare repositories. Please note that --broken and --dirty need a working
> > tree.
> >
> > Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> > ---
> > t/t6120-describe.sh | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> > index 9a6bd1541f..ddd8cc307d 100755
> > --- a/t/t6120-describe.sh
> > +++ b/t/t6120-describe.sh
> > @@ -409,4 +409,11 @@ test_expect_success 'describe complains about missing object' '
> > test_must_fail git describe $ZERO_OID
> > '
> >
> > +test_expect_success 'describe works from outside repo using --git-dir' "
> > + BARE_CLONE=$(mktemp -d) &&
>
> No, keep everything in $TRASH_DIRECTORY so it will be automatically cleaned.
Looks like a relic from trying to get Git to not find an appropriate
working tree.
$TRASH_DIRECTORY/bare works here, too.
>
> > + git clone --bare '$TRASH_DIRECTORY' \$BARE_CLONE >/Users/koraktor/open-source/others/git/t/out &&
>
> Ehh.. I'm pretty sure I don't have /Users/koraktor on my system :)
> This looks like just debug code, I think you can drop ">.." part for
> all commands.
>
Sorry. A bit embarrassing to leave debugging code in place.
I will remove the output redirection and echo.
> > + echo $PWD >/Users/koraktor/open-source/others/git/t/out &&
> > + git --git-dir \$BARE_CLONE describe 2>&1 >/Users/koraktor/open-source/others/git/t/out
> > +"
> > +
> > test_done
> > --
> > 2.20.1
> >
>
>
> --
> Duy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] Add tests for describe with --work-tree
2019-01-27 0:07 ` [PATCH v2 1/3] Add tests for describe with --work-tree Duy Nguyen
@ 2019-01-27 7:13 ` Sebastian Staudt
2019-01-28 10:06 ` Duy Nguyen
0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Staudt @ 2019-01-27 7:13 UTC (permalink / raw)
To: Duy Nguyen, Git Mailing List; +Cc: Junio C Hamano, Jeff King
Am So., 27. Jan. 2019 um 01:07 Uhr schrieb Duy Nguyen <pclouds@gmail.com>:
>
> On Sun, Jan 27, 2019 at 3:51 AM Sebastian Staudt <koraktor@gmail.com> wrote:
> >
> > The dirty ones are already passing, but just because describe is comparing
> > with the wrong working tree.
> >
> > Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> > ---
> > t/t6120-describe.sh | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> > index d639d94696..9a6bd1541f 100755
> > --- a/t/t6120-describe.sh
> > +++ b/t/t6120-describe.sh
> > @@ -28,6 +28,24 @@ check_describe () {
> > '
> > }
> >
> > +check_describe_worktree () {
> > + cd "$TEST_DIRECTORY"
>
> Strange alignment. We normally do it in a subshell...
Sure, will fix this.
>
> > + expect="$1"
> > + shift
> > + R=$(git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe "$@" 2>err.actual)
>
> These commands should be executed inside test_expect_success, not
> outside. And you need to chain commands with && to make sure if
> something breaks, then the whole test will fai.
>
> If it's too ugly to generate test_expect_success with a shell
> function, then just write a shell function that "describe" and compare
> (i.e. the test body). Then you can write something like this later
>
> test_expect_sucesss 'describe with --worktree foo' '
> check_describe_worktree foo
> '
>
> and check_describe_worktree can now do
>
> ( cd "$TEST_DIRECTORY" && .... )
>
>
My function is a modified version of check_describe(). Which does the
same thing. I‘m not really experienced in Shell programming, so I didn‘t
see a cleaner way.
But having the cd commands in the && chain looks broken as it would
break the following tests when one test fails and the code was executed
in the wrong directory afterwards.
>
> > + S=$?
> > + cat err.actual >&3
> > + test_expect_success "describe with --work-tree $*" '
> > + test $S = 0 &&
> > + case "$R" in
> > + $expect) echo happy ;;
> > + *) echo "Oops - $R is not $expect";
> > + false ;;
> > + esac
> > + '
> > + cd "$TRASH_DIRECTORY"
> > +}
> > +
> > test_expect_success setup '
> >
> > test_tick &&
> > @@ -145,14 +163,20 @@ check_describe A-* HEAD
> >
> > check_describe "A-*[0-9a-f]" --dirty
> >
> > +check_describe_worktree "A-*[0-9a-f]" --dirty
> > +
> > test_expect_success 'set-up dirty work tree' '
> > echo >>file
> > '
> >
> > check_describe "A-*[0-9a-f]-dirty" --dirty
> >
> > +check_describe_worktree "A-*[0-9a-f]-dirty" --dirty
> > +
> > check_describe "A-*[0-9a-f].mod" --dirty=.mod
> >
> > +check_describe_worktree "A-*[0-9a-f].mod" --dirty=.mod
> > +
> > test_expect_success 'describe --dirty HEAD' '
> > test_must_fail git describe --dirty HEAD
> > '
> > --
> > 2.20.1
> >
>
>
> --
> Duy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/3] Setup working tree in describe
2019-01-27 0:21 ` Duy Nguyen
@ 2019-01-27 7:19 ` Sebastian Staudt
0 siblings, 0 replies; 11+ messages in thread
From: Sebastian Staudt @ 2019-01-27 7:19 UTC (permalink / raw)
To: Duy Nguyen, Git Mailing List; +Cc: Junio C Hamano, Jeff King
Am So., 27. Jan. 2019 um 01:22 Uhr schrieb Duy Nguyen <pclouds@gmail.com>:
>
> On Sun, Jan 27, 2019 at 3:51 AM Sebastian Staudt <koraktor@gmail.com> wrote:
> >
> > This ensures the given working tree is used for --dirty.
> >
> > The implementation of --broken uses diff-index which calls
> > setup_work_tree() itself.
>
> It would be nice to have a test case covering --broken even if no fix
> is needed (so that somebody else will not accidentally break it
> later). I did a quick test and thought it was broken, but it turns out
> I tested it wrong :P
>
There‘s only one test ("describe ignoring a broken submodule") which
effectively tests --broken.
I could reuse this.
BTW, is "describe ignoring a broken submodule" the right description here?
In fact, this should probably be named "describe detects a broken submodule".
> > Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> > ---
> > builtin/describe.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/builtin/describe.c b/builtin/describe.c
> > index cc118448ee..b5b7abdc8f 100644
> > --- a/builtin/describe.c
> > +++ b/builtin/describe.c
> > @@ -629,6 +629,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
> > struct argv_array args = ARGV_ARRAY_INIT;
> > int fd, result;
> >
> > + setup_work_tree();
> > read_cache();
> > refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED,
> > NULL, NULL, NULL);
> > --
> > 2.20.1
> >
>
>
> --
> Duy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] Add tests for describe with --work-tree
2019-01-27 7:13 ` Sebastian Staudt
@ 2019-01-28 10:06 ` Duy Nguyen
2019-01-30 16:47 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: Duy Nguyen @ 2019-01-28 10:06 UTC (permalink / raw)
To: Sebastian Staudt; +Cc: Git Mailing List, Junio C Hamano, Jeff King
On Sun, Jan 27, 2019 at 08:13:51AM +0100, Sebastian Staudt wrote:
> Am So., 27. Jan. 2019 um 01:07 Uhr schrieb Duy Nguyen <pclouds@gmail.com>:
> >
> > On Sun, Jan 27, 2019 at 3:51 AM Sebastian Staudt <koraktor@gmail.com> wrote:
> > >
> > > The dirty ones are already passing, but just because describe is comparing
> > > with the wrong working tree.
> > >
> > > Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> > > ---
> > > t/t6120-describe.sh | 24 ++++++++++++++++++++++++
> > > 1 file changed, 24 insertions(+)
> > >
> > > diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> > > index d639d94696..9a6bd1541f 100755
> > > --- a/t/t6120-describe.sh
> > > +++ b/t/t6120-describe.sh
> > > @@ -28,6 +28,24 @@ check_describe () {
> > > '
> > > }
> > >
> > > +check_describe_worktree () {
> > > + cd "$TEST_DIRECTORY"
> >
> > Strange alignment. We normally do it in a subshell...
>
> Sure, will fix this.
>
> >
> > > + expect="$1"
> > > + shift
> > > + R=$(git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe "$@" 2>err.actual)
> >
> > These commands should be executed inside test_expect_success, not
> > outside. And you need to chain commands with && to make sure if
> > something breaks, then the whole test will fai.
> >
> > If it's too ugly to generate test_expect_success with a shell
> > function, then just write a shell function that "describe" and compare
> > (i.e. the test body). Then you can write something like this later
> >
> > test_expect_sucesss 'describe with --worktree foo' '
> > check_describe_worktree foo
> > '
> >
> > and check_describe_worktree can now do
> >
> > ( cd "$TEST_DIRECTORY" && .... )
> >
> >
>
> My function is a modified version of check_describe().
Whoa. That function is 12 years old! I think our style has evolved a
bit since then.
> Which does the same thing. I‘m not really experienced in Shell
> programming, so I didn‘t see a cleaner way.
>
> But having the cd commands in the && chain looks broken as it would
> break the following tests when one test fails and the code was executed
> in the wrong directory afterwards.
I mean chaining within a test. This is to make sure any failure
triggers the test failure (as it should, if some command is expected
to fail, we have other ways to catch it).
I would start with something simple, not using shell function at
all. Something like this as an example (I added run_describe() because
that "git" command becomes too long). Have a look at the "do's and
don'ts" in t/README too.
-- 8< --
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index d639d94696..646bedf4e9 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -28,6 +28,10 @@ check_describe () {
'
}
+run_describe() {
+ git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe "$@"
+}
+
test_expect_success setup '
test_tick &&
@@ -145,6 +149,14 @@ check_describe A-* HEAD
check_describe "A-*[0-9a-f]" --dirty
+test_expect_success 'describe with --work-tree --dirty' '
+ (
+ cd "$TEST_DIRECTORY" &&
+ run_describe --dirty 2>err.actual >actual &&
+ grep "^A-.*[0-9a-f]$" actual
+ )
+'
+
test_expect_success 'set-up dirty work tree' '
echo >>file
'
-- 8< --
BTW, careful about _success or _failure. You need to make sure bisect
is not broken. If you add a test to confirm a broken case then it
should be test_expect_failure (and the test suite will pass). Then
when you fix it you can flip it to test_expect_success.
--
Duy
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] Add tests for describe with --work-tree
2019-01-28 10:06 ` Duy Nguyen
@ 2019-01-30 16:47 ` Junio C Hamano
0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2019-01-30 16:47 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Sebastian Staudt, Git Mailing List, Jeff King
Duy Nguyen <pclouds@gmail.com> writes:
>> My function is a modified version of check_describe().
>
> Whoa. That function is 12 years old! I think our style has evolved a
> bit since then.
;-).
> I mean chaining within a test. This is to make sure any failure
> triggers the test failure (as it should, if some command is expected
> to fail, we have other ways to catch it).
>
> I would start with something simple, not using shell function at
> all. Something like this as an example (I added run_describe() because
> that "git" command becomes too long). Have a look at the "do's and
> don'ts" in t/README too.
Thanks for guiding new contributors with an easy to understand example.
> BTW, careful about _success or _failure. You need to make sure bisect
> is not broken. If you add a test to confirm a broken case then it
> should be test_expect_failure (and the test suite will pass). Then
> when you fix it you can flip it to test_expect_success.
And if the fix is simple enough (i.e. a good rule of thumb is if the
fixes themselves without tests need to be multi-patch series, it is
not simple enough), have a single patch that has both fix and test
that expects success, instead of splitting them into two to make a
two patch series whose first step expects a failure and whose second
step fixes and flips failure to success.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-01-30 16:47 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-26 20:49 [PATCH v2 1/3] Add tests for describe with --work-tree Sebastian Staudt
2019-01-26 20:49 ` [PATCH v2 2/3] Setup working tree in describe Sebastian Staudt
2019-01-27 0:21 ` Duy Nguyen
2019-01-27 7:19 ` Sebastian Staudt
2019-01-26 20:49 ` [PATCH v2 3/3] Add test for describe with a bare repository Sebastian Staudt
2019-01-27 0:25 ` Duy Nguyen
2019-01-27 6:54 ` Sebastian Staudt
2019-01-27 0:07 ` [PATCH v2 1/3] Add tests for describe with --work-tree Duy Nguyen
2019-01-27 7:13 ` Sebastian Staudt
2019-01-28 10:06 ` Duy Nguyen
2019-01-30 16:47 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).