Thread: Adjust pg_regress output for new long test names
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
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.
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
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
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.
[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.
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.
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
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.
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?
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
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.