Re: meson missing test dependencies

Hi,

On 2024-12-06 18:54:25 +0100, Peter Eisentraut wrote:
> On 05.12.24 21:10, Andres Freund wrote:
> > On 2024-12-05 20:08:24 +0100, Peter Eisentraut wrote:
> > > On 03.12.24 17:01, Andres Freund wrote:
> > > > On 2024-12-02 11:10:56 +0100, Peter Eisentraut wrote:
> > > > That's unfortunately a very partial fix - because we insist on tests being run
> > > > against a temporary install, we don't just need to rebuild the code, we also
> > > > need to re-install it. Without that you don't, e.g., see server changes.
> > >
> > > Right, I was willing to accept that as a compromise, but see below.
> > >
> > > > However, it looks like the tmp_install test *does* miss dependencies too and I
> > > > see no reason to not fix that.
> > >
> > > > diff --git i/meson.build w/meson.build
> > > > index ff3848b1d85..55b751a0c6b 100644
> > > > --- i/meson.build
> > > > +++ w/meson.build
> > > > @@ -3263,6 +3263,7 @@ test('tmp_install',
> > > > priority: setup_tests_priority,
> > > > timeout: 300,
> > > > is_parallel: false,
> > > > + depends: all_built,
> > > > suite: ['setup'])
> > > >
> > > > test('install_test_files',
> > >
> > > Yes, that addresses my cube example.
> >
> > Let's do that. I'm inclined to only do so on master, but I can also see
> > backpatching it. Arguments?
>
> master is enough for me.

This doesn't actually make that much difference. But only really kinda by
accident, so I'm still planning to apply a patch improving this.

If a test target does not have ay dependency 'meson test' treats it has having
a dependency on the project default test. Which in turn currently includes
everything that will be installed, i.e. everything that tmp_install might
need. But that's not something I think we should rely on going forward.

Attached is a series that:

- Reduces the dependencies of the 'install-quiet' target, to not build
e.g. ecpg tests

- Fixes the dependencies of tmp_install test more precise

- Fixes dependencies for ecpg's tests

- Fixes dependencies for test_json_parser's (slightly evolved version of
yours)

- Fixes dependencies for libpq_pipeline

- Fixes dependencies for libpq's tests

The last case was a bit less fun to fix.

To have correct dependencies, the libpq tests need a dependency on
libpq_uri_regress and libpq_testclient. But until now that was hard to do,
because src/interfaces/libpq/test is entered from the top-level:

# test/ is entered via top-level meson.build, that way it can use the default
# args for executables (which depend on libpq).

At the top-level we have:
subdir('src/interfaces/libpq')
# fe_utils depends on libpq
subdir('src/fe_utils')

# for frontend binaries
frontend_code = declare_dependency(
include_directories: [postgres_inc],
link_with: [fe_utils, common_static, pgport_static],
sources: generated_headers,
dependencies: [os_deps, libintl],
)

and the test binaries currently use frontend_code.

The least bad fix that I could see was to introduce, at the top-level,
frontend_no_fe_utils_code, which can be defined before we get to libpq/. That
in turn allows us to recurse into libpq/test from within libpq/meson.build,
which allows us to define the tests with proper dependencies.

With all of these applied
ninja clean && ninja meson-test-prereq && m test --no-rebuild

passes, which doesn't guarantee that *all* dependencies are correctly
declared, but certainly is better than where we were before.

Greetings,

Andres Freund