Re: Add extension options to control TAP and isolation tests

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Nikolay Shaplov <dhyan(at)nataraj(dot)su>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Adam Berlin <berlin(dot)ab(at)gmail(dot)com>
Subject: Re: Add extension options to control TAP and isolation tests
Date: 2018-11-21 00:39:53
Message-ID: 20181121003953.GD1951@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Nov 20, 2018 at 07:30:39PM +0300, Nikolay Shaplov wrote:
> Is it ok, if I join the reviewing? I like test, especially TAP one, you know
> ;-)
>
> Since you are much more experienced in postgres then me, I'd try to
> understand how does the patch work, try to use if for writing more TAP
> test, and will report problems and thoughts I came across while doing that.

Thanks for bumping in the field.

> For me name "output_iso" means nothing. iso is something about CD/DVD or about
> standards. I would not guess that iso stands for isolation if I did not know
> it already. isolation_output is more sensible: I have heard that there are
> some isolation tests, this must be something about it. May be it would be
> better to change it to isolation_output everywhere instead of changing to
> output_iso

That's just a default configuration used by the isolation tester.
That's not much bothering with in my opinion for this patch, as the
patch is here to make sure that the default configuration gets used
where it had better be used. Changing this default value would be of
course doable technically, but that's around for years to changing it
does not seem like good idea.

> I tried to find definition in documentation what does "isolation test" exactly
> means, but did not find. There is some general words about TAP tests in main
> postgres documentation
> https://www.postgresql.org/docs/11/regress-tap.html,
> but I would not understand anything from it if I did not already know how it
> works.

Those are mentioned here as part of the additional test suites:
https://www.postgresql.org/docs/11/regress-run.html#id-1.6.20.5.5

> In current extend-pgxs documentation there is some explanation about
> regression test, it sensible enough. Since TAP and isolation tests are
> introduced now, there should be same short explanation for both of
> them.

I see your point here, and it is true that documentation ought to be
better. So I have written out a new set of paragraphs which explain the
whereabouts of the new variables a bit more in depth in the section of
extend-pgxs.

> And also it would be good to add links from extend-pgxs to regress-tap and
> regress saying that for more info about these tests one can look at postgres
> doc, because they work in a similar way.

I have added a reference to regress-tap in one of the new paragraphs.
Linking the existing stuff to point to "regress" is a separate issue
though, and while pointing to the TAP section is adapted as its
guidelines are rather general, I am not sure which one would make the
most sense though.
--
Michael

Attachment Content-Type Size
pgxs-isolation-tap-v2.patch text/x-diff 13.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2018-11-21 01:32:23 Re: [RFC] Removing "magic" oids
Previous Message Andres Freund 2018-11-21 00:07:59 Re: [RFC] Removing "magic" oids