[PATCH v1 1/1] tests: add Git submodule version consistency test

[Ferry Huberts]

On 19/03/13 11:43, John Keeping wrote:
> 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
> '
>


Did you test this?
Because the submodule SHA points to the commit, not to the tag.
So the GIT_VER and submodule SHA are actually different.
That is the reason I did the git diff

It appears your script-fu is better than mine ;-)

>
> John
>

-- 
Ferry Huberts