Re: Add extension options to control TAP and isolation tests

В письме от 10 ноября 2018 09:14:19 пользователь Michael Paquier написал:

> > Nice cleanup. Also, I like the ability to enable/control more types of
> > tests easily from the Makefile. What are the next steps for this
> > patch?
>
> Thanks. It seems to me that a complete review is still in order,
> particularly regarding the new makefile option names. And then, if
> everybody caring about the topic is happy with the way the patch is
> shaped, it can be carried over to being committed, which would be most
> likely something I'll do.

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.

So far while first reading the patch I came to following two

1.
--- a/src/test/modules/brin/.gitignore
+++ b/src/test/modules/brin/.gitignore
@@ -1,3 +1,3 @@
# Generated subdirectories
-/isolation_output/
+/output_iso/
/tmp_check/

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

2.

--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1293,6 +1293,34 @@ include $(PGXS)
</listitem>
</varlistentry>
.
+ <varlistentry>
+ <term><varname>ISOLATION</varname></term>
+ <listitem>
+ <para>
+ list of isolation test cases
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><varname>ISOLATION_OPTS</varname></term>
+ <listitem>
+ <para>
+ additional switches to pass to
+ <application>pg_isolation_regress</application>
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><varname>TAP_TESTS</varname></term>
+ <listitem>
+ <para>
+ switch defining if TAP tests need to be run
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><varname>NO_INSTALLCHECK</varname></term>
<listitem>

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.

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.

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.

That's all so far. I'll look more into it later...

--
Do code for fun.