Re: [PATCHv4 1/2] git-p4: add test case for RCS keywords

Luke Diamand [off-list ref] writes:

+create_kw_file() {
+ cat <<'EOF' > $1

Please start this shell function like this instead:

        create_kw_file () {
                cat <<\EOF >"$1"

The first line is merely cosmetic but matches the convention used in our
shell scripts better.

As to the second line:

 * Quoting "$1" should not be strictly necessary according to POSIX rules,
   but we saw reports that some shells "helpfully" give unnecessary
   warnings when $1 (or whatever variable that holds the name of the file
   the output is redirected into) contains $IFS characters.

 * An indent in our shell scripts is a HT, not a SP (or four spaces or
   whatever).

 * And our convention is not to have an extra SP after redirection '>' (or
   '<') operator.

+test_expect_success 'init depot' '
+	(
+		cd "$cli" &&
+		echo file1 >file1 &&
+		p4 add file1 &&
+		p4 submit -d "change 1" &&
+		create_kw_file kwfile1.c &&
+		p4 add kwfile1.c &&
+		p4 submit -d "Add rcw kw file" kwfile1.c
+	)
+'
+
+p4_append_to_file() {
+ f=$1
+ p4 edit -t ktext $f &&
+ echo "/* $(date) */" >> $f &&
+ p4 submit -d "appending a line in p4" &&
+ cat $f
+}

Surround all occurrences of '$f' with double-quotes, e.g.

	cat "$f"

to avoid unexpected breakage when later somebody tries to use a file whose
name happens to contain a space. The first assignment "f=$1" can stay as-is.

+
+# Create some files with RCS keywords. If they get modified
+# elsewhere then the version number gets bumped which then
+# results in a merge conflict if we touch the RCS kw lines,
+# even though the change itself would otherwise apply cleanly.
+test_expect_failure 'cope with rcs keyword expansion damage' '
+ "$GITP4" clone --dest="$git" //depot &&
+ cd "$git" &&

Please do not cd around in test script outside a subshell.  Otherwise, any
failure in a subsequent command in this && chain before you reach the next
"cd" will leave you in an unexpected directory and can break later tests.

The worst example would be

	test_expect_something 'one' '
		mkdir -p git/play/pen &&
                cd git/play/pen &&
                do something &&
        	cd ../../.. &&
                do something that potentially can fail &&
                cd git/play/pen &&
                do something
	'
	# blindly assuming the above succeeded up to the last 'cd git'
        test_expect_something 'two' '
		cd ../../.. &&
        	rm -fr git &&
                do something else
	'

If the first test fails in the middle before it brings you back down to
'git/play/pen' with the last 'cd git/playpen', then the next test moves
you up three levels and you will be running "rm -fr" on a wrong 'git'!