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?
Attachments:
0001-Adjust-pg_regress-output-for-long-test-names.patchapplication/x-patch; name=0001-Adjust-pg_regress-output-for-long-test-names.patchDownload
From 129b4a7c7993508d09718cb5ca326ab0db6af342 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Wed, 9 Jun 2021 13:31:38 +1200
Subject: [PATCH] Adjust pg_regress output for long test names.
Make the test output visually consistent, as previously done by commit
14378245.
---
src/test/regress/pg_regress.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index e04d365258..20ea07dc0f 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1738,7 +1738,7 @@ run_schedule(const char *schedule, test_start_function startfunc,
if (num_tests == 1)
{
- status(_("test %-28s ... "), tests[0]);
+ status(_("test %-32s ... "), tests[0]);
pids[0] = (startfunc) (tests[0], &resultfiles[0], &expectfiles[0], &tags[0]);
INSTR_TIME_SET_CURRENT(starttimes[0]);
wait_for_tests(pids, statuses, stoptimes, NULL, 1);
@@ -1794,7 +1794,7 @@ run_schedule(const char *schedule, test_start_function startfunc,
bool differ = false;
if (num_tests > 1)
- status(_(" %-28s ... "), tests[i]);
+ status(_(" %-32s ... "), tests[i]);
/*
* Advance over all three lists simultaneously.
@@ -1893,7 +1893,7 @@ run_single_test(const char *test, test_start_function startfunc,
*tl;
bool differ = false;
- status(_("test %-28s ... "), test);
+ status(_("test %-32s ... "), test);
pid = (startfunc) (test, &resultfiles, &expectfiles, &tags);
INSTR_TIME_SET_CURRENT(starttime);
wait_for_tests(&pid, &exit_status, &stoptime, NULL, 1);
--
2.30.2
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 msAny 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
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.
Show quoted text
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-BARI 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 msAny 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-BARI 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 havepartition-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.