git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
* [PATCH] t/perf: rename duplicate-numbered test script
@ 2019-08-12 15:58 Jeff King
  2019-08-12 16:04 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff King @ 2019-08-12 15:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

There are two perf scripts numbered p5600, but with otherwise different
names ("clone-reference" versus "partial-clone"). We store timing
results in files named after the whole script, so internally we don't
get confused between the two. But "aggregate.perl" just prints the test
number for each result, giving multiple entries for "5600.3". It also
makes it impossible to skip one test but not the other with
GIT_SKIP_TESTS.

Let's renumber the one that appeared later (by date -- the source of the
problem is that the two were developed on independent branches). For the
non-perf test suite, our test-lint rule would have complained about this
when the two were merged, but t/perf never learned that trick.

Signed-off-by: Jeff King <peff@peff.net>
---
This is meant for 2.23, but obviously it's not hurting anything if it
doesn't make the cut. I double-checked that there is no conflict with
anything on pu, either. :)

In other news, I ran the whole perf suite on v2.23.0-rc2, and there was
nothing interesting aside from the expected improvement from 39b44ba771
(check_everything_connected: assume alternate ref tips are valid,
2019-07-01).

 t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} (100%)

diff --git a/t/perf/p5600-clone-reference.sh b/t/perf/p5601-clone-reference.sh
similarity index 100%
rename from t/perf/p5600-clone-reference.sh
rename to t/perf/p5601-clone-reference.sh
-- 
2.23.0.rc2.466.gd4688353c7

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

* Re: [PATCH] t/perf: rename duplicate-numbered test script
  2019-08-12 15:58 [PATCH] t/perf: rename duplicate-numbered test script Jeff King
@ 2019-08-12 16:04 ` Junio C Hamano
  2019-08-12 16:13   ` Jeff King
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2019-08-12 16:04 UTC (permalink / raw)
  To: Jeff King; +Cc: git

Jeff King <peff@peff.net> writes:

> There are two perf scripts numbered p5600, but with otherwise different
> names ("clone-reference" versus "partial-clone"). We store timing
> results in files named after the whole script, so internally we don't
> get confused between the two. But "aggregate.perl" just prints the test
> number for each result, giving multiple entries for "5600.3". It also
> makes it impossible to skip one test but not the other with
> GIT_SKIP_TESTS.
>
> Let's renumber the one that appeared later (by date -- the source of the
> problem is that the two were developed on independent branches). For the
> non-perf test suite, our test-lint rule would have complained about this
> when the two were merged, but t/perf never learned that trick.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This is meant for 2.23, but obviously it's not hurting anything if it
> doesn't make the cut. I double-checked that there is no conflict with
> anything on pu, either. :)

Thanks for being careful.  Will apply.

>  t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} (100%)
>
> diff --git a/t/perf/p5600-clone-reference.sh b/t/perf/p5601-clone-reference.sh
> similarity index 100%
> rename from t/perf/p5600-clone-reference.sh
> rename to t/perf/p5601-clone-reference.sh

By the way, do we feel differently (e.g. more risky) when we see
100% rename without the "index old-oid..new-oid mode" lines and when
we see 99% rename with one, with a one-line change?

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

* Re: [PATCH] t/perf: rename duplicate-numbered test script
  2019-08-12 16:04 ` Junio C Hamano
@ 2019-08-12 16:13   ` Jeff King
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff King @ 2019-08-12 16:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Mon, Aug 12, 2019 at 09:04:36AM -0700, Junio C Hamano wrote:

> >  t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} | 0
> >  1 file changed, 0 insertions(+), 0 deletions(-)
> >  rename t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} (100%)
> >
> > diff --git a/t/perf/p5600-clone-reference.sh b/t/perf/p5601-clone-reference.sh
> > similarity index 100%
> > rename from t/perf/p5600-clone-reference.sh
> > rename to t/perf/p5601-clone-reference.sh
> 
> By the way, do we feel differently (e.g. more risky) when we see
> 100% rename without the "index old-oid..new-oid mode" lines and when
> we see 99% rename with one, with a one-line change?

I saw that earlier message from Linus, too. :)

For a change like this, I don't think it matters either way. Whatever is
the content of that file, my intent is to move it to a new location. So
if you did have changes, moving them along with it would be the right
thing.

That said, I'm not at all opposed to having more data in the patch. Even
if the "apply" side doesn't do anything useful with the "index" line in
such a case, it's possible it could help with tracking down a mis-merge
or other confusion after the fact.

What I don't think we would want, though, is for a change on your side
to reject a patch like mine (a sort of "RENAME/MODIFY" conflict, I
guess, where "am" says "I can't move 1234abcd to this new filename,
because that's not what I have at that path"). Our merges are already
happy to port changes around when there's a rename on another branch,
and this case is no different.

-Peff

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-12 15:58 [PATCH] t/perf: rename duplicate-numbered test script Jeff King
2019-08-12 16:04 ` Junio C Hamano
2019-08-12 16:13   ` Jeff King

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox