[PATCH/RFC 00/24] Re: [PATCH 1/3] t9300 (fast-import): style tweaks

Hi,

Jonathan Nieder wrote:

... many things ...

... 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(-)