From mboxrd@z Thu Jan  1 00:00:00 1970
From: john at keeping.me.uk (John Keeping)
Date: Tue, 19 Mar 2013 10:43:32 +0000
Subject: [PATCH v1 1/1] tests: add Git submodule version consistency test
In-Reply-To: <1363651305-30614-1-git-send-email-mailings@hupie.com>
References: <1363651305-30614-1-git-send-email-mailings@hupie.com>
Message-ID: <20130319104332.GA2283@serenity.lan>

On Tue, Mar 19, 2013 at 01:01:45AM +0100, Ferry Huberts wrote:
> From: Ferry Huberts <ferry.huberts at pelagic.nl>
> 
> To ensure the versions are in sync
> 
> Signed-off-by: Ferry Huberts <ferry.huberts at pelagic.nl>
> ---
>  tests/t0001-validate-git-versions.sh | 63 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100755 tests/t0001-validate-git-versions.sh
> 
> diff --git a/tests/t0001-validate-git-versions.sh b/tests/t0001-validate-git-versions.sh
> new file mode 100755
> index 0000000..22066c0
> --- /dev/null
> +++ b/tests/t0001-validate-git-versions.sh
> @@ -0,0 +1,63 @@
> +#!/bin/sh
> +
> +. ./setup.sh
> +
> +
> +test_versions()
> +{
> +	curdir=`pwd`

Please use $(pwd) rather than backticks.

> +	cd ..

If you cd, we should do so in a subshell to avoid leaving $PWD in the
wrong place if we exit early.  I don't see any need to cd for all the
tests here though, the only time we care is when we need to find the Git
submodule version.

> +
> +	gitSubmoduleStatus=`git submodule status git`

Generally variables are written_like_this, not in camelCase.

> +	echo "gitSubmoduleStatus          = $gitSubmoduleStatus"
> +
> +	gitSubmoduleStatusFirstChar=`echo "$gitSubmoduleStatus" | cut -c -1`
> +	echo "gitSubmoduleStatusFirstChar = $gitSubmoduleStatusFirstChar"
> +
> +	# Fail the test if the Git submodule is not initialised
> +	test "$gitSubmoduleStatusFirstChar" == "-" && \
> +		echo "The Git submodule is not initialised" && \
> +		return 1 

I don't know if we need these diagnostics, can't we just get the two
versions and output the difference?

> +
> +	# Fail the test if the Git submodule is not clean
> +	test "$gitSubmoduleStatusFirstChar" != " " && \
> +		echo "The Git submodule is not clean" && \
> +		return 1
> +
> +	# Get the SHA1 from the (clean) Git submodule
> +	gitSubmoduleVersionSha=`git submodule status git| awk '{ print $1 }' | cut -c 1-`
> +	echo "gitSubmoduleVersionSha      = $gitSubmoduleVersionSha"
> +
> +	# Extract the Git version of the archive from the Makefile
> +	regex='^[[:space:]]*GIT_VER[[:space:]]*=[[:space:]]*(.*)[[:space:]]*$'
> +	archiveVersion=`grep -E "$regex" Makefile | sed -r "s/$regex/\1/"`

We can just use sed here, there's no need for grep as well:

    sed -n -e '/^GIT_VER[ 	]*=/ {
        s/^GIT_VER[ 	]*=[ 	]*//
        p
    }' Makefile

(Anyone who puts leading spaces before variables in the Makefile
deserves what they get here ;-) )

> +	echo "archiveVersion              = $archiveVersion"
> +
> +	# Compare the Git submodule version and the Makefile Git version
> +	cd git
> +	git diff --exit-code --quiet "$gitSubmoduleVersionSha..v$archiveVersion"
> +	diffExitCode=$?
> +	echo "diffExitCode                = $diffExitCode"
> +	cd ..
> +
> +	# Return to the original directory
> +	cd "$curdir"
> +	
> +	# Determine exit code
> +	test $diffExitCode -ne 0 && return 1
> +	return 0
> +}
> +
> +
> +prepare_tests 'Check Git version consistency'
> +
> +git=`which git`
> +test -n "$git" || {
> +	echo "Skipping test: git not found"
> +	tests_done
> +	exit
> +}

I don't think any of the tests will run without Git (how can they create
tests repositories?) so this is unnecessary.

> +
> +run_test 'test versions' 'test_versions'

The test should be inline in run_test, there's no need for the
test_versions function.

> +
> +tests_done


I like the idea of this, but I think we should be able to get away with
something much simpler like this:

run_test 'Git versions are consistent' '
	( cd ../git && git describe || echo "No submodule!" ) >tmp/sm_version &&
	sed -n -e "/^GIT_VER[ 	]*=/ {
		s/^GIT_VER[ 	]*=[ 	]*//
		p
	}" >tmp/make_version &&
	diff -u tmp/sm_version tmp/make_version
'


John