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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html