Re: pgsql: Provide a TLS init hook

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: pgsql: Provide a TLS init hook

Andrew Dunstan-8

[discussion from -committers]


On 3/26/20 4:31 PM, Tom Lane wrote:

> Andrew Dunstan <[hidden email]> writes:
>> On 3/26/20 11:31 AM, Tom Lane wrote:
>>> Andrew Dunstan <[hidden email]> writes:
>>>> I don't think this belongs in installcheck, we should add
>>>> 'NO_INSTALLCHECK = 1' to the Makefile.
>>> Why?  The other src/test/modules/ modules with TAP tests do not
>>> specify that, with the exception of commit_ts which has a solid
>>> doesnt-work-in-the-default-configuration excuse.
>> That seems wrong, installcheck should be testing against an installed
>> instance, and the TAP tests don't.
> So?  We clearly document that for the TAP tests, "make installcheck"
> means "use the installed executables, but run a new instance" [1].



I think we were probably a bit shortsighted about that. But what's done
is done. I wonder if there is a simple way we could turn it off for the
buildfarm?



>> Moreover, from the buildfarm's POV
>> it's completely wrong, as we call the installcheck targets multiple
>> times, once for each configured locale. See one of the animals that
>> tests multiple locales (e.g. crake or prion)
> Yeah.  That's productive if you think the tests might be
> locale-sensitive.  I doubt that any of the ones under src/test/modules/
> actually are at the moment, so maybe this is a waste of buildfarm effort.
> But I don't think that it's the place of the Makefiles to dictate such
> policy, and especially not for them to do so by breaking the ability to
> use "make installcheck" at all.
>
> regards, tom lane
>
> [1] https://www.postgresql.org/docs/devel/regress-tap.html


Right now the explicit TAP test code in the buildfarm knows how to collect all the relevant output. The installcheck code doesn't know about that for TAP tests.

I get that developers want to be able to run tests in a small number of commands, but for the buildfarm I generally favor more disaggregated tests. That way if test X fails it's much easier to focus on the problem. (related note: I'm working on breaking up the log text blobs which will also help focussing on the right area).

Maybe we need to take the discussion to -hackers.


cheers

andrew


--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Reply | Threaded
Open this post in threaded view
|

Re: pgsql: Provide a TLS init hook

Tom Lane-2
Andrew Dunstan <[hidden email]> writes:
> [discussion from -committers]

> On 3/26/20 4:31 PM, Tom Lane wrote:
>> So?  We clearly document that for the TAP tests, "make installcheck"
>> means "use the installed executables, but run a new instance" [1].

> I think we were probably a bit shortsighted about that. But what's done
> is done. I wonder if there is a simple way we could turn it off for the
> buildfarm?

I think it was entirely intentional.  I use "installcheck" all the time
to save the cost of repeatedly building an install tree.  If anything,
it's more important to be able to do that when running a specific
subdirectory's tests than when testing the whole tree, because the
overhead would be worse in proportion.  So sprinkling NO_INSTALLCHECK
liberally would make me sad.  (In fact, I wonder if we should treat
that as only disabling traditional-framework tests not TAP tests.
The problem of tests requiring atypical configurations doesn't apply
to TAP tests.)

> Right now the explicit TAP test code in the buildfarm knows how to collect all the relevant output. The installcheck code doesn't know about that for TAP tests.

It seems like what the buildfarm would like is a way to invoke TAP tests
and traditional-framework tests separately, so that it could apply special
tooling to the former.  I'd have no objection to making that possible.

Alternatively, maybe you could just dig through the tree after-the-fact
looking for tmp_check subdirectories, and capturing their contents?

A separate issue is whether or not it's worth running all the
src/test/modules/ tests over again for multiple locales.  ISTM the
answer is mostly "no", but I grant that some of them might be locale
sensitive.  Maybe we could mark the ones that are in their Makefiles?
Get the buildfarm to look for "LOCALE_SENSITIVE = 1" or the like.
Right now, the modules/ tests don't run long enough for this to be super
important, but we might be more worried about their cost in future.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: pgsql: Provide a TLS init hook

Andrew Dunstan-8

On 3/27/20 11:09 AM, Tom Lane wrote:
>
>> Right now the explicit TAP test code in the buildfarm knows how to collect all the relevant output. The installcheck code doesn't know about that for TAP tests.
> It seems like what the buildfarm would like is a way to invoke TAP tests
> and traditional-framework tests separately, so that it could apply special
> tooling to the former.  I'd have no objection to making that possible.
>

Exactly. I'll look into that, but I'm open to any ideas people have.


cheers


andrew

--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Reply | Threaded
Open this post in threaded view
|

Re: pgsql: Provide a TLS init hook

Tom Lane-2
Andrew Dunstan <[hidden email]> writes:
> On 3/27/20 11:09 AM, Tom Lane wrote:
>> It seems like what the buildfarm would like is a way to invoke TAP tests
>> and traditional-framework tests separately, so that it could apply special
>> tooling to the former.  I'd have no objection to making that possible.

> Exactly. I'll look into that, but I'm open to any ideas people have.

With the makefile infrastructure, the first thing that comes to mind
is to support something like

        make [install]check SKIP_TAP_TESTS=1

        make [install]check SKIP_TRADITIONAL_TESTS=1

Don't know what to do in the MSVC world.

                        regards, tom lane