Horton

Thread

Commits

  1. Remove dependency to pageinspect in recovery tests

  1. "make installcheck" fails in src/test/recovery

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-04-18T05:45:45Z

    Up until quite recently, it worked to do "make installcheck" in
src/test/recovery, following the instructions in the README
file there:

    NOTE: You must have given the --enable-tap-tests argument to configure.
    Also, to use "make installcheck", you must have built and installed
    contrib/test_decoding in addition to the core code.
    
    Run
        make check
    or
        make installcheck
    You can use "make installcheck" if you previously did "make install".
    In that case, the code in the installation tree is tested.  With
    "make check", a temporary installation tree is built from the current
    sources and then tested.

Now, however, the 016_min_consistency.pl test is falling over,
with symptoms indicating that it expects to have the pageinspect
extension installed as well:

error running SQL: 'psql:<stdin>:2: ERROR:  could not open extension control fil
e "/home/postgres/testversion/share/extension/pageinspect.control": No such file
 or directory'
while running 'psql -XAtq -d port=64106 host=/tmp/KaoBFubKfw dbname='postgres' -
f - -v ON_ERROR_STOP=1' with sql '
CREATE EXTENSION pageinspect;
...

Is this extra dependency actually essential?  I'm not really
happy about increasing the number of moving parts in this test.

			regards, tom lane

  2. Re: "make installcheck" fails in src/test/recovery

    Michael Paquier <michael@paquier.xyz> — 2019-04-19T00:27:07Z

    On Thu, Apr 18, 2019 at 01:45:45AM -0400, Tom Lane wrote:
> Is this extra dependency actually essential?  I'm not really
> happy about increasing the number of moving parts in this test.

Hmmm.  I don't actually object to removing the part depending on
pageinspect in the tests.  Relying on the on-disk page format has
proved to be more reliable for the buildfarm than I initially
thought, and we are actually able to keep the same coverage without
the dependency on pageinspect.

Now, I don't think that this is not a problem only for
src/test/recovery/ but to any path using EXTRA_INSTALL.  For example,
if you take contrib/ltree_plpython/, then issue "make install" from
this path followed by an installcheck, then the tests complain about
ltree missing from the installation.  For the recovery tests, we
already require test_decoding so I would expect the problem to get
worse with the time as we should not restrict the dependencies with
other modules if they make sense for some TAP tests.

I am wondering if it would be better to just install automatically all
the paths listed in EXTRA_INSTALL when invoking installcheck.  We
enforce the target in src/test/recovery/Makefile, still we could use
this opportunity to mark it with TAP_TESTS=1.
--
Michael

  3. Re: "make installcheck" fails in src/test/recovery

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-04-19T01:31:21Z

    Michael Paquier <michael@paquier.xyz> writes:
> I am wondering if it would be better to just install automatically all
> the paths listed in EXTRA_INSTALL when invoking installcheck.

Absolutely not!  In the first place, "make installcheck" is supposed to
test the installed tree, not editorialize upon what's in it; and in the
second place, you wouldn't necessarily have permissions to change that
tree.

If we think that depending on pageinspect is worthwhile here, the right
thing to do is just to fix the README to say that you need it.  I'm
merely asking whether it's really worth the extra dependency.

			regards, tom lane

  4. Re: "make installcheck" fails in src/test/recovery

    Michael Paquier <michael@paquier.xyz> — 2019-04-19T02:23:04Z

    On Thu, Apr 18, 2019 at 09:31:21PM -0400, Tom Lane wrote:
> If we think that depending on pageinspect is worthwhile here, the right
> thing to do is just to fix the README to say that you need it.  I'm
> merely asking whether it's really worth the extra dependency.

The dependency is not strongly mandatory for the test coverage.  I'll
just go remove it if that's an annoyance.
--
Michael

  5. Re: "make installcheck" fails in src/test/recovery

    Michael Paquier <michael@paquier.xyz> — 2019-04-19T06:57:46Z

    On Fri, Apr 19, 2019 at 11:23:04AM +0900, Michael Paquier wrote:
> The dependency is not strongly mandatory for the test coverage.  I'll
> just go remove it if that's an annoyance.

And done.
--
Michael