git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Avoid duplicate test number t7609
@ 2010-12-13 10:14 Johannes Sixt
  2010-12-13 16:12 ` [PATCH/RFC] t800?-blame.sh: retitle uniquely Michael J Gruber
  0 siblings, 1 reply; 7+ messages in thread
From: Johannes Sixt @ 2010-12-13 10:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

From: Johannes Sixt <j6t@kdbg.org>

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/{t7609-merge-abort.sh => t7611-merge-abort.sh} |    0
 1 files changed, 0 insertions(+), 0 deletions(-)
 rename t/{t7609-merge-abort.sh => t7611-merge-abort.sh} (100%)

diff --git a/t/t7609-merge-abort.sh b/t/t7611-merge-abort.sh
similarity index 100%
rename from t/t7609-merge-abort.sh
rename to t/t7611-merge-abort.sh
-- 
1.7.3.3.1800.g26f22

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH/RFC] t800?-blame.sh: retitle uniquely
  2010-12-13 10:14 [PATCH] Avoid duplicate test number t7609 Johannes Sixt
@ 2010-12-13 16:12 ` Michael J Gruber
  2010-12-13 17:07   ` Jeff King
  0 siblings, 1 reply; 7+ messages in thread
From: Michael J Gruber @ 2010-12-13 16:12 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Sixt

Currently we have three test files matching t800?-blame.sh.

Rename the latter two to make it easier to spot where additions would
go.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
On a related note to J6t's patch, how's the stance on unique titles?
Unique numbers are important for partial test runs, of course,
but unique titles help finding you way through the test.
There are more than the blame.sh ones.

 t/{t8003-blame.sh => t8003-blame-corner-cases.sh}  |    0
 ...8004-blame.sh => t8004-blame-with-conflicts.sh} |    0
 2 files changed, 0 insertions(+), 0 deletions(-)
 rename t/{t8003-blame.sh => t8003-blame-corner-cases.sh} (100%)
 rename t/{t8004-blame.sh => t8004-blame-with-conflicts.sh} (100%)

diff --git a/t/t8003-blame.sh b/t/t8003-blame-corner-cases.sh
similarity index 100%
rename from t/t8003-blame.sh
rename to t/t8003-blame-corner-cases.sh
diff --git a/t/t8004-blame.sh b/t/t8004-blame-with-conflicts.sh
similarity index 100%
rename from t/t8004-blame.sh
rename to t/t8004-blame-with-conflicts.sh
-- 
1.7.3.3.738.g018bc

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH/RFC] t800?-blame.sh: retitle uniquely
  2010-12-13 16:12 ` [PATCH/RFC] t800?-blame.sh: retitle uniquely Michael J Gruber
@ 2010-12-13 17:07   ` Jeff King
  2010-12-13 17:22     ` Jeff King
  2010-12-13 19:51     ` Junio C Hamano
  0 siblings, 2 replies; 7+ messages in thread
From: Jeff King @ 2010-12-13 17:07 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Junio C Hamano, Johannes Sixt

On Mon, Dec 13, 2010 at 05:12:29PM +0100, Michael J Gruber wrote:

> On a related note to J6t's patch, how's the stance on unique titles?
> Unique numbers are important for partial test runs, of course,
> but unique titles help finding you way through the test.
> There are more than the blame.sh ones.

I don't think it is a big deal, but I did just 5 minutes ago get annoyed
at:

  t7500-commit.sh
  t7501-commit.sh
  t7502-commit.sh
  t7509-commit.sh

Speaking of minor test issues, we should probably also do this:

-- >8 --
Subject: [PATCH] tests: flip executable bit on t9158

All tests are supposed to be executable.

Signed-off-by: Jeff King <peff@peff.net>
---
 0 files changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 t/t9158-git-svn-mergeinfo.sh

diff --git a/t/t9158-git-svn-mergeinfo.sh b/t/t9158-git-svn-mergeinfo.sh
old mode 100644
new mode 100755
-- 
1.7.3.3.784.gccc31.dirty

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH/RFC] t800?-blame.sh: retitle uniquely
  2010-12-13 17:07   ` Jeff King
@ 2010-12-13 17:22     ` Jeff King
  2010-12-13 19:38       ` Junio C Hamano
  2010-12-13 19:51     ` Junio C Hamano
  1 sibling, 1 reply; 7+ messages in thread
From: Jeff King @ 2010-12-13 17:22 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Junio C Hamano, Johannes Sixt

On Mon, Dec 13, 2010 at 12:07:34PM -0500, Jeff King wrote:

> Speaking of minor test issues, we should probably also do this:
> 
> -- >8 --
> Subject: [PATCH] tests: flip executable bit on t9158

BTW, I will plug my test-lint patch once again, which caught both this
and the duplicate test number mentioned earlier.

-- >8 --
Subject: [PATCH] tests: add some script lint checks

There are some common but minor errors we tend to make in
writing test scripts:

  1. Scripts are left non-executable. This is not usually
     noticed immediately because "make test" does not need
     the bit, but it is a matter of git policy to make them
     executable (and is a slight convenience when running
     individual scripts).

  2. Two scripts are allocated the same number. Usually this
     happens on separate branches, and the problem only
     comes about during a merge. But since there is no
     textual conflict, the merger would have to be very
     observant to notice.

     This is also a minor error, but can make GIT_SKIP_TESTS
     ambiguous.

This patch introduces a "test-lint" target which checks
both. It is not invoked by default. You can invoke it as
"make test-lint", or you can make it a prerequisite of
running the tests by specifying "TEST_LINT = test-lint" in
your config.mak or on the command line.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/Makefile |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/t/Makefile b/t/Makefile
index 73c6ec4..47cbeb6 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -23,10 +23,10 @@ TGITWEB = $(wildcard t95[0-9][0-9]-*.sh)
 
 all: $(DEFAULT_TEST_TARGET)
 
-test: pre-clean
+test: pre-clean $(TEST_LINT)
 	$(MAKE) aggregate-results-and-cleanup
 
-prove: pre-clean
+prove: pre-clean $(TEST_LINT)
 	@echo "*** prove ***"; GIT_CONFIG=.git/config $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
 	$(MAKE) clean
 
@@ -41,6 +41,18 @@ clean:
 	$(RM) -r valgrind/bin
 	$(RM) .prove
 
+test-lint: test-lint-duplicates test-lint-executable
+
+test-lint-duplicates:
+	@dups=`echo $(T) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
+		test -z "$$dups" || { \
+		echo >&2 "duplicate test numbers:" $$dups; exit 1; }
+
+test-lint-executable:
+	@bad=`for i in $(T); do test -x "$$i" || echo $$i; done` && \
+		test -z "$$bad" || { \
+		echo >&2 "non-executable tests:" $$bad; exit 1; }
+
 aggregate-results-and-cleanup: $(T)
 	$(MAKE) aggregate-results
 	$(MAKE) clean
-- 
1.7.3.3.784.gccc31.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH/RFC] t800?-blame.sh: retitle uniquely
  2010-12-13 17:22     ` Jeff King
@ 2010-12-13 19:38       ` Junio C Hamano
  0 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2010-12-13 19:38 UTC (permalink / raw)
  To: Jeff King; +Cc: Michael J Gruber, git, Johannes Sixt

Thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH/RFC] t800?-blame.sh: retitle uniquely
  2010-12-13 17:07   ` Jeff King
  2010-12-13 17:22     ` Jeff King
@ 2010-12-13 19:51     ` Junio C Hamano
  2010-12-13 19:58       ` Jeff King
  1 sibling, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2010-12-13 19:51 UTC (permalink / raw)
  To: Michael J Gruber, Jeff King; +Cc: git, Johannes Sixt

Jeff King <peff@peff.net> writes:

> On Mon, Dec 13, 2010 at 05:12:29PM +0100, Michael J Gruber wrote:
>
>> On a related note to J6t's patch, how's the stance on unique titles?
>> Unique numbers are important for partial test runs, of course,
>> but unique titles help finding you way through the test.
>> There are more than the blame.sh ones.
>
> I don't think it is a big deal, but I did just 5 minutes ago get annoyed
> at:
>
>   t7500-commit.sh
>   t7501-commit.sh
>   t7502-commit.sh
>   t7509-commit.sh

t7509 seems to be about the authorship, so it is easy to rename it to
t7509-commit-authorship or something, but unfortunately I do not see
unifying theme in any of t750[012].  They test random things and there
seem to be overlaps.

Perhaps somebody wants to consolidate these three into one?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH/RFC] t800?-blame.sh: retitle uniquely
  2010-12-13 19:51     ` Junio C Hamano
@ 2010-12-13 19:58       ` Jeff King
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff King @ 2010-12-13 19:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael J Gruber, git, Johannes Sixt

On Mon, Dec 13, 2010 at 11:51:58AM -0800, Junio C Hamano wrote:

> >   t7500-commit.sh
> >   t7501-commit.sh
> >   t7502-commit.sh
> >   t7509-commit.sh
> 
> t7509 seems to be about the authorship, so it is easy to rename it to
> t7509-commit-authorship or something, but unfortunately I do not see
> unifying theme in any of t750[012].  They test random things and there
> seem to be overlaps.

I read through them and came to the same conclusion.

> Perhaps somebody wants to consolidate these three into one?

I think this falls into my "would be nice if it had been written cleaner
in the first place, but is not worth the time to clean up" category. But
if somebody else is willing to work on it, I have no objection. :)

-Peff

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-12-13 19:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-13 10:14 [PATCH] Avoid duplicate test number t7609 Johannes Sixt
2010-12-13 16:12 ` [PATCH/RFC] t800?-blame.sh: retitle uniquely Michael J Gruber
2010-12-13 17:07   ` Jeff King
2010-12-13 17:22     ` Jeff King
2010-12-13 19:38       ` Junio C Hamano
2010-12-13 19:51     ` Junio C Hamano
2010-12-13 19:58       ` Jeff King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).