Thread: Adjust pg_regress output for new long test names

Adjust pg_regress output for new long test names

From
Thomas Munro
Date:
Hi,

test deadlock-simple              ... ok           20 ms
test deadlock-hard                ... ok        10624 ms
test deadlock-soft                ... ok          147 ms
test deadlock-soft-2              ... ok         5154 ms
test deadlock-parallel            ... ok          132 ms
test detach-partition-concurrently-1 ... ok          553 ms
test detach-partition-concurrently-2 ... ok          234 ms
test detach-partition-concurrently-3 ... ok         2389 ms
test detach-partition-concurrently-4 ... ok         1876 ms

Any objections to making these new tests line up with the rest?

Attachment

Re: Adjust pg_regress output for new long test names

From
Julien Rouhaud
Date:
On Wed, Jun 09, 2021 at 01:57:45PM +1200, Thomas Munro wrote:
> Hi,
> 
> test deadlock-simple              ... ok           20 ms
> test deadlock-hard                ... ok        10624 ms
> test deadlock-soft                ... ok          147 ms
> test deadlock-soft-2              ... ok         5154 ms
> test deadlock-parallel            ... ok          132 ms
> test detach-partition-concurrently-1 ... ok          553 ms
> test detach-partition-concurrently-2 ... ok          234 ms
> test detach-partition-concurrently-3 ... ok         2389 ms
> test detach-partition-concurrently-4 ... ok         1876 ms
> 
> Any objections to making these new tests line up with the rest?

No objection, as the output is still way under 80 characters.



Re: Adjust pg_regress output for new long test names

From
Michael Paquier
Date:
On Wed, Jun 09, 2021 at 10:17:35AM +0800, Julien Rouhaud wrote:
> On Wed, Jun 09, 2021 at 01:57:45PM +1200, Thomas Munro wrote:
>> Any objections to making these new tests line up with the rest?
>
> No objection, as the output is still way under 80 characters.

+1.
--
Michael

Attachment

Re: Adjust pg_regress output for new long test names

From
Tom Lane
Date:
Thomas Munro <thomas.munro@gmail.com> writes:
> test detach-partition-concurrently-1 ... ok          553 ms
> test detach-partition-concurrently-2 ... ok          234 ms
> test detach-partition-concurrently-3 ... ok         2389 ms
> test detach-partition-concurrently-4 ... ok         1876 ms

> Any objections to making these new tests line up with the rest?

... or we could shorten those file names.  I recall an episode
awhile ago where somebody complained that their version of "tar"
couldn't handle some of the path names in our tarball, so
keeping things from getting to carpal-tunnel-inducing lengths
does have its advantages.

            regards, tom lane



Re: Adjust pg_regress output for new long test names

From
Noah Misch
Date:
On Wed, Jun 09, 2021 at 01:57:45PM +1200, Thomas Munro wrote:
> test deadlock-simple              ... ok           20 ms
> test deadlock-hard                ... ok        10624 ms
> test deadlock-soft                ... ok          147 ms
> test deadlock-soft-2              ... ok         5154 ms
> test deadlock-parallel            ... ok          132 ms
> test detach-partition-concurrently-1 ... ok          553 ms
> test detach-partition-concurrently-2 ... ok          234 ms
> test detach-partition-concurrently-3 ... ok         2389 ms
> test detach-partition-concurrently-4 ... ok         1876 ms

> Make the test output visually consistent, as previously done by commit
> 14378245.

Not bad, but I would instead shorten the names to detach-[1234] or
detach-partition-[1234].  The marginal value of the second word is low, and
the third word helps even less.

> -            status(_("test %-28s ... "), tests[0]);
> +            status(_("test %-32s ... "), tests[0]);

As the whitespace gulf widens, it gets harder to match left and right sides
visually.  We'd cope of course, but wider spacing isn't quite free.



Re: Adjust pg_regress output for new long test names

From
Thomas Munro
Date:
[Responding to two emails in one]

On Wed, Jun 9, 2021 at 2:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> ... or we could shorten those file names.  I recall an episode
> awhile ago where somebody complained that their version of "tar"
> couldn't handle some of the path names in our tarball, so
> keeping things from getting to carpal-tunnel-inducing lengths
> does have its advantages.

On Wed, Jun 9, 2021 at 2:51 PM Noah Misch <noah@leadboat.com> wrote:
> Not bad, but I would instead shorten the names to detach-[1234] or
> detach-partition-[1234].  The marginal value of the second word is low, and
> the third word helps even less.

Alright, CC'ing Alvaro who added the long names to see if he wants to
consider that.

There's one other case of this phenomenon:
tuplelock-upgrade-no-deadlock overflows by one character.



Re: Adjust pg_regress output for new long test names

From
Noah Misch
Date:
On Wed, Jun 09, 2021 at 03:21:36PM +1200, Thomas Munro wrote:
> On Wed, Jun 9, 2021 at 2:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > ... or we could shorten those file names.  I recall an episode
> > awhile ago where somebody complained that their version of "tar"
> > couldn't handle some of the path names in our tarball, so
> > keeping things from getting to carpal-tunnel-inducing lengths
> > does have its advantages.
> 
> On Wed, Jun 9, 2021 at 2:51 PM Noah Misch <noah@leadboat.com> wrote:
> > Not bad, but I would instead shorten the names to detach-[1234] or
> > detach-partition-[1234].  The marginal value of the second word is low, and
> > the third word helps even less.

Better still, the numbers can change to something descriptive:

detach-1 => detach-visibility
detach-2 => detach-fk-FOO
detach-3 => detach-incomplete
detach-4 => detach-fk-BAR

I don't grasp the difference between -2 and -4 enough to suggest concrete FOO
and BAR words.

> Alright, CC'ing Alvaro who added the long names to see if he wants to
> consider that.



Re: Adjust pg_regress output for new long test names

From
Alvaro Herrera
Date:
On 2021-Jun-08, Noah Misch wrote:

> On Wed, Jun 09, 2021 at 03:21:36PM +1200, Thomas Munro wrote:
> > On Wed, Jun 9, 2021 at 2:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > ... or we could shorten those file names.  I recall an episode
> > > awhile ago where somebody complained that their version of "tar"
> > > couldn't handle some of the path names in our tarball, so
> > > keeping things from getting to carpal-tunnel-inducing lengths
> > > does have its advantages.

Sure.  I'm also the author of tuplelock-upgrade-no-deadlock -- see
commit de87a084c0a5.  (Oleksii submitted it as "rowlock-upgrade-deadlock").
We could rename that one too while at it.

> > On Wed, Jun 9, 2021 at 2:51 PM Noah Misch <noah@leadboat.com> wrote:
> > > Not bad, but I would instead shorten the names to detach-[1234] or
> > > detach-partition-[1234].  The marginal value of the second word is low, and
> > > the third word helps even less.
> 
> Better still, the numbers can change to something descriptive:
> 
> detach-1 => detach-visibility
> detach-2 => detach-fk-FOO
> detach-3 => detach-incomplete
> detach-4 => detach-fk-BAR
> 
> I don't grasp the difference between -2 and -4 enough to suggest concrete FOO
> and BAR words.

Looking at -2, it looks like a very small subset of -4.  I probably
wrote it first and failed to realize I could extend that one rather than
create -4.  We could just delete it.

We also have partition-concurrent-attach.spec; what if we make
everything a consistent set?  We could have

partition-attach
partition-detach-visibility (-1)
partition-detach-incomplete (-3)
partition-detach-fk         (-4)

-- 
Álvaro Herrera                            39°49'30"S 73°17'W



Re: Adjust pg_regress output for new long test names

From
Peter Eisentraut
Date:
On 09.06.21 04:51, Noah Misch wrote:
> On Wed, Jun 09, 2021 at 01:57:45PM +1200, Thomas Munro wrote:
>> test deadlock-simple              ... ok           20 ms
>> test deadlock-hard                ... ok        10624 ms
>> test deadlock-soft                ... ok          147 ms
>> test deadlock-soft-2              ... ok         5154 ms
>> test deadlock-parallel            ... ok          132 ms
>> test detach-partition-concurrently-1 ... ok          553 ms
>> test detach-partition-concurrently-2 ... ok          234 ms
>> test detach-partition-concurrently-3 ... ok         2389 ms
>> test detach-partition-concurrently-4 ... ok         1876 ms
>> Make the test output visually consistent, as previously done by commit
>> 14378245.
> Not bad, but I would instead shorten the names to detach-[1234] or
> detach-partition-[1234].  The marginal value of the second word is low, and
> the third word helps even less.

DETACH CONCURRENTLY is a separate feature from plain DETACH.

But "partition" is surely redundant here.



Re: Adjust pg_regress output for new long test names

From
Peter Eisentraut
Date:
On 09.06.21 03:57, Thomas Munro wrote:
> test deadlock-simple              ... ok           20 ms
> test deadlock-hard                ... ok        10624 ms
> test deadlock-soft                ... ok          147 ms
> test deadlock-soft-2              ... ok         5154 ms
> test deadlock-parallel            ... ok          132 ms
> test detach-partition-concurrently-1 ... ok          553 ms
> test detach-partition-concurrently-2 ... ok          234 ms
> test detach-partition-concurrently-3 ... ok         2389 ms
> test detach-partition-concurrently-4 ... ok         1876 ms
> 
> Any objections to making these new tests line up with the rest?

Can we scan all the test names first and then pick a suitable length?





Re: Adjust pg_regress output for new long test names

From
Robert Haas
Date:
On Wed, Jun 9, 2021 at 1:37 PM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
> Can we scan all the test names first and then pick a suitable length?

FWIW, I think this discussion of shortening the test case names is
probably going in the wrong direction. It's true that in many cases
such a thing can be done, but it's also true that the test case
authors picked those names because they felt that they described those
test cases well. It's not unlikely that future test case authors will
have similar feelings and will again pick names that are a little bit
longer. It's also not impossible that in shortening the names we will
make them less clear. For example, Peter said that "partition" was
redundant in something like "detach-partition-concurrently-4," but
that is only true if you think that a partition is the only thing that
can be detached. That is true today as far as the SQL grammar is
concerned, but from a source code perspective we speak of detaching
from shm_mq objects or DSMs, and there could be more things, internal
or SQL-visible, in the future.

Now I don't care all that much; this isn't worth getting worked up
about. But if it were me, I'd tend to err in the direction of
accommodating longer test names, and only shorten them if it's clear
that someone *really* went overboard.

-- 
Robert Haas
EDB: http://www.enterprisedb.com



Re: Adjust pg_regress output for new long test names

From
Noah Misch
Date:
On Wed, Jun 09, 2021 at 09:31:24AM -0400, Alvaro Herrera wrote:
> On 2021-Jun-08, Noah Misch wrote:
> > > On Wed, Jun 9, 2021 at 2:51 PM Noah Misch <noah@leadboat.com> wrote:
> > > > Not bad, but I would instead shorten the names to detach-[1234] or
> > > > detach-partition-[1234].  The marginal value of the second word is low, and
> > > > the third word helps even less.
> > 
> > Better still, the numbers can change to something descriptive:
> > 
> > detach-1 => detach-visibility
> > detach-2 => detach-fk-FOO
> > detach-3 => detach-incomplete
> > detach-4 => detach-fk-BAR
> > 
> > I don't grasp the difference between -2 and -4 enough to suggest concrete FOO
> > and BAR words.
> 
> Looking at -2, it looks like a very small subset of -4.  I probably
> wrote it first and failed to realize I could extend that one rather than
> create -4.  We could just delete it.
> 
> We also have partition-concurrent-attach.spec; what if we make
> everything a consistent set?  We could have
> 
> partition-attach
> partition-detach-visibility (-1)
> partition-detach-incomplete (-3)
> partition-detach-fk         (-4)

That works for me.  I'd be fine with Peter Eisentraut's tweaks, too.