Re: [HACKERS] pgbench tap tests & minor fixes - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: [HACKERS] pgbench tap tests & minor fixes
Date
Msg-id alpine.DEB.2.20.1704251810070.10770@lancre
Whole thread Raw
In response to Re: [HACKERS] pgbench tap tests & minor fixes  (Nikolay Shaplov <dhyan@nataraj.su>)
Responses Re: [HACKERS] pgbench tap tests & minor fixes.  (Nikolay Shaplov <dhyan@nataraj.su>)
List pgsql-hackers
Hello,

>> Nope. TestLib does not know about PostgresNode, the idea is rather that
>> PostgresNode knows and wraps around TestLib when needed.
>
> Actually as I look at this part, I feeling an urge to rewrite this code, and
> change it so, that all command_like calls were called in a context of certain
> node, but it is subject for another patch. In this patch it would be good to
> use TestLib functions as they are now, or with minimum modifications.

I do not understand. In the v2 I sent ISTM that I did exactly as it is 
done for other test functions:
 - the test function itself is in TestLib
 - PostgresNode wraps around it to provide the necessary connection   information which it is holding.

Maybe a better pattern could be thought of, but at least now my addition 
is consistent with pre-existing code.

>> Now I can add a command_likes which allows to manage status, stdout and
>> stderr and add that in TestLib & PostgresNode.

> This is good idea too, I still do not understand why do you want to add it to
> PostgresNode.

There is a command_like function in TestLib and a method of the same name 
in PostgresNode to provide the function with connections informations.

> And name command_likes -- a very bad solution, as it can easily be confused
> with command_like.

That is a good point. What other name do you suggest?

> If it is possible to do it backward compatible, I would try
> to improve command_like, so it can check all the results. If it is not, I
> would give this function a name that brings no confusion.

The problem I have with the pre-existing functions is that they do a 
partial job: whether status is ok nor not, whether stdout contains 
something XOR whether stderr contains something. I want to do all that 
repeatedly, hence the enhanced version which checks all three with list of 
patterns.

Now maybe I can extend the existing "command_like" to check for extra 
arguments, to get the expected status and manage list of patterns for both 
stdout & stderr instead of a single regex, but for me this is somehow a 
distinct function.

>>>> Hmmm. I thought of that but was very unconvinced by the approach: 
>>>> pgbench basically always requires a file, so what the stuff was doing 
>>>> was writting the script into a file, checking for possible errors 
>>>> when doing so, then unlinking the file and checking for errors again 
>>>> after the run.
>>>
>>> I think I was wrong about deleting file after tests are finished. Better
>>> keep them.
>>
>> Hmmm... Then what is the point not to have them as files to begin with?
>
> Not to have them in source code tree in git.

I do not see that as a problem, the point of git is to manage file 
contents anyway.

Now, as I said, I will write unreadable code if required, I will only be 
sad about it.

> The rest would be in the answer to another sum up letter.

Ok.

-- 
Fabien.



pgsql-hackers by date:

Previous
From: Serge Rielau
Date:
Subject: Re: [HACKERS] Cached plans and statement generalization
Next
From: Stephen Frost
Date:
Subject: Re: [HACKERS] pg_dump emits ALTER TABLE ONLY partitioned_table