From: Jonathan Nieder <jrnieder@gmail.com>
Date: Fri, 24 Sep 2010 06:59:00 GMT
Subject: [PATCH/RFC 00/24] Re: [PATCH 1/3] t9300 (fast-import): style tweaks
Message-ID: <20100924065900.GA4666@burratino>
In-Reply-To: <20100905032253.GB2344@burratino>

Hi,

Jonathan Nieder wrote:

> Clarify dependencies between tests to make the fast-import test
> script more approachable.  In particular:
... many things ...
> While at it:
... more things ...

The patch was a lazy way for me to add new assertions to the
fast-import test script without going crazy.  But it really was lazy:
it has almost nothing to do with the "fast-import protocol
experiments" series that it headed, and worse, that one patch did so
many things at once that it was basically guaranteed that (1) no one
would like all of it and (2) it bitrotted in a couple of days.

Oh well.  Tomorrow I would like to re-roll the fast-import experiment
so the svn-fe that understands deltas can get more attention, and of
course that series does not require these style fixes at all.

So why resend them?  I end up mentally making these changes every time
I add a new test to that script, so I imagine it would be nicer to
make the changes once.  Maybe it would help newcomers to dive into the
wonderful world of fast-import testing.

So here is a small chunk of that monster patch, ejected from the
original series and split up.

Patch 1 introduces a verify_packs () helper that makes the script much
easier to read (by including only two copies of an unpleasant loop).
The nominal justification is that giving the

	for each pack
	do
		git verify-pack $pack ||
		exit
	done

loop its own function allows use to write "return" instead of "exit",
resulting in better behavior when a test fails.

Patch 2 is the most important one to me.  It gets rid of some
hardcoded tree and blob names, most of which were not doing any
harm except to scare me.  At first glance it is not obvious when
a stray test_tick, for example, will ruin later tests (it turns out
never but there is at one test that does depend on the choice of
hash function), and at first glance, it is not obvious what is
actually the expected diff when a raw diff is presented as expected
output.

The approach adopted is to introduce some symbolic constants, like

	empty_blob=$(git hash-object --stdin </dev/null)

and use them in the expected output.

Patches 3-6 just pick nits.  The dividends are better output
with -v and more robust checking for failure of git commands.

Patch 7 changes some 4-space indents to tabs (since the latter
is predominant in the file).

Patches 8-24 change from traditional

	test_expect_failure \
		'description' \
		'commands &&
		 more commands'

to modern

	test_expect_success 'description' '
		commands &&
		more commands
	'

style.  They are meant to be squashed together and are only split
in tiny pieces for easier review.

Let's take whatever is useful and forget about the rest.

Thanks,
Jonathan Nieder (24):
  t9300 (fast-import): avoid exiting early on failure
  t9300 (fast-import): avoid hard-coded object names
  t9300 (fast-import): guard "export large marks" test
  t9300 (fast-import): check exit status from upstream of pipes
  t9300 (fast-import): check exit status from command substitution
  t9300 (fast-import): use test_cmp in place of test $(foo) = $(bar)
  t9300 (fast-import): use tabs to indent
  t9300 (fast-import), series A: re-indent
  t9300 (fast-import), series B: re-indent
  t9300 (fast-import), series C: re-indent
  t9300 (fast-import), series D: re-indent
  t9300 (fast-import), series E: re-indent
  t9300 (fast-import), series F: re-indent
  t9300 (fast-import), series H: re-indent
  t9300 (fast-import), series I: re-indent
  t9300 (fast-import), series J: re-indent
  t9300 (fast-import), series K: re-indent
  t9300 (fast-import), series L: re-indent
  t9300 (fast-import), series M: re-indent
  t9300 (fast-import), series N: re-indent
  t9300 (fast-import), series O: re-indent
  t9300 (fast-import), series P: re-indent
  t9300 (fast-import), series Q: re-indent
  t9300 (fast-import), series R: re-indent

 t/t9300-fast-import.sh | 1010 ++++++++++++++++++++++++++----------------------
 1 files changed, 539 insertions(+), 471 deletions(-)

-- 
1.7.2.3