Test script style (Re: [PATCH 08/18] git notes merge: Initial implementation handling trivial merges only)

[Jonathan Nieder <jrnieder@gmail.com>]

Johan Herland wrote:

> This initial implementation of 'git notes merge' only handles the trivial
> merge cases (i.e. where the merge is either a no-op, or a fast-forward).

Mm, sounds like a nice thing to have anyway.

Now to digress: some generalities about test scripts.  This will
probably be very tedious.  I'm writing it as groundwork before
reviewing your test (since I don't feel on solid ground about this
topic at all).

More focused thoughts on other aspects of the patch will follow in a
separate message.

First:

	See http://thread.gmane.org/gmane.comp.version-control.git/155596/focus=155673
	If you don't like what you see, no need to read the rest
	of this message. :)

Motivation:

	Not being the best of shell script readers (or writers,
	for that matter), I need all the help from stylistic cues
	I can get.  Old test scripts have a consistent style
	generally, but newer ones are starting to diverge from
	one another.

A rough test format:

	#!/bin/sh
	#
	# Copyright (c) 2010 ...
	#

	test_description='...
	. ./test-lib.sh
	. ...

	constant data
	function definitions

	setup }
	test  }
	test  } repeat as necessary
	test  }

	test_done

Test description:

	test_description = 'One-line description

	Some words or diagrams describing the invariants (e.g., history)
	maintained between tests'

Constant data:

	Simple commands to produce test vectors, expected output, and
	other variables and files that it might be convenient to have
	unchanged available throughout the test script.

	Because not guarded by a test assertion, this section would
	not include any git commands, nor any commands that might fail
	or write to the terminal.  So: this section might use
	"cat <<\", "mkdir -p", "cp", "cp -R", "mv", "printf", "foo=",
	"unset", and "export", but not much else.

Function definitions:

	Tests can define functions inside a test_expect_success block,
	too, but the generally useful functions would go up front with
	the constant data.

Setup:

	test_expect_success '(setup | set up) ...' '
		Commands to set up for the next batch of tests.
		Can rely on previous setup and can do just about
		anything.
	'

Tests:
	test_expect_success|failure '... some claim...' '
		Commands to test that claim.
		Could do all sorts of things, as long as they do not
		disturb the invariants established by the
		constant_data and setup sections.
	'

What is not present:

	A test in this style would not include any commands except for
	test_done outside a test assertion after the first
	test_expect_success.

This is meant to be a sort of compromise between what t/README says
("Put all code inside test_expect_success and other assertions") and
what people actually do.  I have no problem with people doing
something else --- in fact, some tests would not make any sense in
this style.  In general, if a person is writing maintainable tests
that do not slow down "make test" by much, it does not make much sense
to put new obstacles in her way.

What I expect is that new tests would be easier to read and extend in
this style, since to get started, all a person has to pay attention to
are the setup commands.  So a potential collaborator could say "if you
use this structure, it will make my life easier".

Thoughts?  Would it make sense to eventually put something like this
in t/CodingStyle or nearby?