Thread (42 messages) flat view 42 messages, 5 authors, 2016-06-15
STALE3740d

[PATCH 0/5] modernize test style

From: Tom Grennan <hidden>
Date: 2016-06-15 22:53:11

Tom Grennan [off-list ref] writes:
On Tue, Feb 21, 2012 at 10:33:29PM -0800, Junio C Hamano wrote:
quoted
I know you are imitating the style of surrounding tests that is an older
parts of this script, but it is an eyesore.  More modern tests are written
like this:

test_expect_success 'label for the test' '
	cat >expect <<-EOF &&
               v0.2.1
	EOF
        git tag -l ... >actual &&
	test_cmp expect actual
'

to avoid unnecessary backslash on the first line, and have the preparation
of test vectore _inside_ test_expect_success.  We would eventually want to
update the older part to the newer style for consistency.

Two possible ways to go about this are (1) have a "pure style" patch at
the beginning to update older tests to a new style and then add new code
and new test as a follow-up patch written in modern, or (2) add new code
and new test in modern, and make a mental note to update the older ones
after the dust settles.  Adding new tests written in older style to a file
that already has mixed styles is the worst thing you can do.

This comment applies to all the patches in this series with tests.
I'd prefer, (1) precede each "--exclude" patch with a "pure style" patch
to update the respective tests.  However, since this will result in a
lot of conflict with concurrent development;  I'll separate the test
patches from the code and documentation.  I'll then cycle on rebasing
the style and new test patches until the development of each is
quiescent.
Per request, the following series modernize the style of the respective
test scripts.  The common themes are:
	- Guard setup with test_expect_success
	- Single-quoted, tab prefaced test blocks of < 80 cols
	- Redirect unwanted output
	- Use a "here" filter for some expect generation

I also used pipelines to validate expected results rather than temporary
files, i.e.
	TEST | test_cmp expect -
vs.	TEST >actual && test_cmp expect actual

Since the later three patches have a lot of whitespace change, I've included an
alternate, PATCH-w series that filters these for more substantive review.
However, even the filtered series is very large causing me to second guess
whether such style modernization should be pursued; so, I look forward to your
input.

Thanks,
Tom Grennan (5):
  t6300 (for-each-ref): modernize style
  t5512 (ls-remote): modernize style
  t3200 (branch): modernize style
  t0040 (parse-options): modernize style
  t7004 (tag): modernize style

 t/t7004-tag.sh | 1680 ++++++++++++++++++++++++++------------------------------
 1 files changed, 783 insertions(+), 897 deletions(-)

-- 
1.7.8
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help