Answering both emails at once

-----
BackgroundPsql.pm

> While debugging that I got annoyed that a match failure results
> in a timeout exit with absolutely no data logged about what output
> the test got.  So v3-0001 also changes timeout() --- which creates
> a timeout that aborts the test --- to timer() --- which does what
> the test author clearly expected, namely just stop waiting for
> more input.  (There's a thread somewhere around here about making
> that change more globally, but I went ahead and did it here.)

I've found your thread about this - [1], and I agree, using
timer() is better here, we get the stdout and stderr of a timed-out
query

> On second thought, that's far more complicated than necessary.
> It should be sufficient to insert quote marks in the echo commands.

Quote marks really work! Very elegant change, it works with or without
readline - I like it! v3 approach with \set was good too, but I too
prefer quote marks

Also, thanks for making both "pump until" blocks identical, it seemed
a little strange to have them be different. At first I thought it was
made deliberately, but when I was debugging pump_until function I saw
how two streams were filling and it seems that both "pump until"
blocks work the same way

-----
030_pager.pl

> As for 0002, I agree that we can't do much except not insist that
> the match cover the whole line.  However, then the question is how
> much risk are we taking of a false match?  It's not too bad for the
> first three tests, where we know what the query will output so we
> can be sure that the pattern will (or won't) appear if the pager
> is or isn't invoked.  However, testing for just "\d+\n" seems fairly
> scary from that standpoint.  It happens not to match to the current
> contents of information_schema.referential_constraints, but that's
> a very hairy query that's subject to change.  I think we had better
> take this test out of the business of relying on the contents of that
> view, and instead create our own simple view that we can be sure of.

I like the idea of using a hand-crafted view for the test instead of
information_schema.referential_constraints, since its d+ output can
change between PostgreSQL versions, and hand-crafted view won't change
suddenly, only deliberately

> On a more nitpicky level, writing "^.*" is useless, because it'll
> match anything at all, so we might as well just remove it from the
> pattern.

Well, yeah, there's no difference between "^.*39" and just plain "39"
for such a regex

All in all - v4 seems to be both elegant and robust

[1] - https://www.postgresql.org/message-id/[email protected]
 
--
Regards,
Oleg

Reply via email to