Re: [PATCH 1/4] test-suite: adding a test for fast-export with tag variants

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 23, 2009, 00:39 UTC
Message-ID: <7vfxh59in9.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1237758620-6116-1-git-send-email-kusmabite@gmail.com>

Erik Faye-Lund <kusmabite@gmail.com> writes:

Show 8 quoted lines

> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
>  t/t9302-fast-export-tags.sh |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)
>  create mode 100644 t/t9302-fast-export-tags.sh
>
> diff --git a/t/t9302-fast-export-tags.sh b/t/t9302-fast-export-tags.sh
> new file mode 100644

Make it executable if you need to add a new script, but shouldn't these small tests be done as an addition to existing t9301, not as a brand new script?

Show 29 quoted lines

> index 0000000..2ecac32
> --- /dev/null
> +++ b/t/t9302-fast-export-tags.sh
> @@ -0,0 +1,25 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2009 Erik Faye-Lund
> +#
> +
> +test_description='git fast-export tag variants'
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +	touch dummy &&
> +	git add dummy &&
> +	git commit -m "initial commit" &&
> +	HEAD_TREE=`git show -s --pretty=raw HEAD | grep tree | sed "s/tree //"` &&
> +	git tag    tree_tag        -m "tagging a tree" $HEAD_TREE &&
> +	git tag -a tree_tag-obj    -m "tagging a tree" $HEAD_TREE &&
> +	git tag    tag-obj_tag     -m "tagging a tag" tree_tag-obj &&
> +	git tag -a tag-obj_tag-obj -m "tagging a tag" tree_tag-obj
> +'
> +
> +test_expect_success 'tree_tag'        'git fast-export tree_tag'
> +test_expect_success 'tree_tag-obj'    'git fast-export tree_tag-obj'
> +test_expect_success 'tag-obj_tag'     'git fast-export tag-obj_tag'
> +test_expect_success 'tag-obj_tag-obj' 'git fast-export tag-obj_tag-obj'
> +
> +test_done

The purpose of the first patch that adds tests is to expose existing problems, and it is better to say test_expect_failure in them. Later patch to fix these issues will contain code change and also change to flip some of the expect_failure to expect_success, and that way we can see what issue is fixed with which patch more easily.

These tests seem to only care about fast-export not dying, but don't we also want to check if they produce correct results?