{"thread":{"id":"23425","subject":"[PATCH] tests: add some script lint checks","startedAt":"2010-04-12T06:52:34Z","lastAt":"2010-04-12T07:41:45Z","messageCount":2,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"139311","messageId":"20100412065234.GA12462@coredump.intra.peff.net","threadId":"23425","inReplyTo":null,"subject":"[PATCH] tests: add some script lint checks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-12T06:52:34Z","receivedAt":"2010-04-12T06:52:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are some common but minor errors we tend to make in\nwriting test scripts:\n\n  1. Scripts are left non-executable. This is not usually\n     noticed immediately because \"make test\" does not need\n     the bit, but it is a matter of git policy to make them\n     executable (and is a slight convenience when running\n     individual scripts).\n\n  2. Two scripts are allocated the same number. Usually this\n     happens on separate branches, and the problem only\n     comes about during a merge. But since there is no\n     textual conflict, the merger would have to be very\n     observant to notice.\n\n     This is also a minor error, but can make GIT_SKIP_TESTS\n     ambiguous.\n\nThis patch introduces a \"test-lint\" target which checks\nboth. It is not invoked by default. You can invoke it as\n\"make test-lint\", or you can make it a prerequisite of\nrunning the tests by specifying \"TEST_LINT = test-lint\" in\nyour config.mak or on the command line.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe dup half is based on Johannes' earlier patch.\n\nObviously this can help submitters avoid dumb mistakes, so I plan to\nturn it on for myself.  As maintainer, Junio, you might want to run at\nleast with TEST_LINT = test-lint-duplicate, since you can then catch\nduplicates introduced by merges and fix them up as part of the merge.\n\n t/Makefile |   14 +++++++++++++-\n 1 files changed, 13 insertions(+), 1 deletions(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 25c559b..40ae67c 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -16,7 +16,7 @@ SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)\n TSVN = $(wildcard t91[0-9][0-9]-*.sh)\n \n-all: pre-clean\n+all: pre-clean $(TEST_LINT)\n \t$(MAKE) aggregate-results-and-cleanup\n \n $(T):\n@@ -30,6 +30,18 @@ clean:\n \t$(RM) t????/cvsroot/CVSROOT/?*\n \t$(RM) -r valgrind/bin\n \n+test-lint: test-lint-duplicates test-lint-executable\n+\n+test-lint-duplicates:\n+\t@dups=`echo $(T) | tr ' ' '\\n' | sed 's/-.*//' | sort | uniq -d` && \\\n+\t\ttest -z \"$$dups\" || { \\\n+\t\techo >&2 \"duplicate test numbers:\" $$dups; exit 1; }\n+\n+test-lint-executable:\n+\t@bad=`for i in $(T); do test -x \"$$i\" || echo $$i; done` && \\\n+\t\ttest -z \"$$bad\" || { \\\n+\t\techo >&2 \"non-executable tests:\" $$bad; exit 1; }\n+\n aggregate-results-and-cleanup: $(T)\n \t$(MAKE) aggregate-results\n \t$(MAKE) clean\n-- \n1.7.1.rc1.245.g808fe\n"},{"id":"139312","messageId":"7vochpm81y.fsf@alter.siamese.dyndns.org","threadId":"23425","inReplyTo":"20100412065234.GA12462@coredump.intra.peff.net","subject":"Re: [PATCH] tests: add some script lint checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-12T07:41:45Z","receivedAt":"2010-04-12T07:41:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> As maintainer, Junio, you might want to run at\n> least with TEST_LINT = test-lint-duplicate, since you can then catch\n> duplicates introduced by merges and fix them up as part of the merge.\n\nThanks.  They are caught by pre-applypatch from 'todo' these days.  I\nshould also perhaps check for the executable bit there.\n"}]}