Thread: Re: meson missing test dependencies

Re: meson missing test dependencies

From
Nazir Bilal Yavuz
Date:
Hi,

On Mon, 2 Dec 2024 at 13:11, Peter Eisentraut <peter@eisentraut.org> wrote:
>
> I have noticed that under meson many tests don't have dependencies on
> the build artifacts that they are testing.  As an example among many, if
> you make a source code change in contrib/cube/cube.c (see patch 0001 for
> a demo) and then run
>
>      make -C contrib/cube check
>
> the test run will reflect the changed code, because the "check" targets
> typically depend on the "all" targets.  But if you do this under meson with
>
>      meson test -C build --suite setup --suite cube
>
> the code will not be rebuilt first, and the test run will not reflect
> the changed code.
>
> This seems straightforward to fix, see patch 0002.  The meson test setup
> has support for this, but it seems not widely used.
>
> Patch 0003 is another example, this time for a TAP-style test.
>
> Is there any reason this was not done yet?

This looks useful. I am not sure why this was not done before.

I applied your patches and the cube example worked. But when I edited
'test_json_parser_perf.c' and 'test_json_parser_incremental.c', meson
did not rebuild. I used the 'meson test -C build --suite setup --suite
test_json_parser' command to test test_json_parser. Did I do something
wrong?

-- 
Regards,
Nazir Bilal Yavuz
Microsoft



Re: meson missing test dependencies

From
Peter Eisentraut
Date:
On 02.12.24 11:50, Nazir Bilal Yavuz wrote:
> I applied your patches and the cube example worked. But when I edited
> 'test_json_parser_perf.c' and 'test_json_parser_incremental.c', meson
> did not rebuild. I used the 'meson test -C build --suite setup --suite
> test_json_parser' command to test test_json_parser. Did I do something
> wrong?

Seems to work for me. I edited test_json_parser_incremental.c and then ran

$ meson test -C build --suite setup --suite test_json_parser -v
ninja: Entering directory `/Users/peter/devel/postgresql/postgresql/build'
[2/2] Linking target 
src/test/modules/test_json_parser/test_json_parser_incremental
1/7 postgresql:setup / tmp_install 
            RUNNING
...

Without my patch, you don't get the "Linking target ..." output.





Re: meson missing test dependencies

From
Nazir Bilal Yavuz
Date:
Hi,

On Mon, 2 Dec 2024 at 22:27, Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 02.12.24 11:50, Nazir Bilal Yavuz wrote:
> > I applied your patches and the cube example worked. But when I edited
> > 'test_json_parser_perf.c' and 'test_json_parser_incremental.c', meson
> > did not rebuild. I used the 'meson test -C build --suite setup --suite
> > test_json_parser' command to test test_json_parser. Did I do something
> > wrong?
>
> Seems to work for me. I edited test_json_parser_incremental.c and then ran
>
> $ meson test -C build --suite setup --suite test_json_parser -v
> ninja: Entering directory `/Users/peter/devel/postgresql/postgresql/build'
> [2/2] Linking target
> src/test/modules/test_json_parser/test_json_parser_incremental
> 1/7 postgresql:setup / tmp_install
>             RUNNING
> ...
>
> Without my patch, you don't get the "Linking target ..." output.

It is working fine now, I must have done something wrong before. Sorry
for the noise.


--
Regards,
Nazir Bilal Yavuz
Microsoft



Re: meson missing test dependencies

From
Nazir Bilal Yavuz
Date:
Hi,

On Tue, 3 Dec 2024 at 04:05, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Dec 02, 2024 at 01:50:57PM +0300, Nazir Bilal Yavuz wrote:
> > On Mon, 2 Dec 2024 at 13:11, Peter Eisentraut <peter@eisentraut.org> wrote:
> >> Is there any reason this was not done yet?
> >
> > This looks useful. I am not sure why this was not done before.
>
> Do you think that it would be possible to automate the addition of the
> dependency link between the module and the tests in some way?  The
> most common case where the only dependency needed by the test is the
> module itself would cover a lot of ground if this could be enforced in
> some fashion.

I tried something like:

diff --git a/meson.build b/meson.build
index 451c3f6d851..ddf6045d472 100644
--- a/meson.build
+++ b/meson.build
@@ -3366,6 +3366,13 @@ foreach test_dir : tests

     t = test_dir[kind]

+    t_dep = [get_variable(test_dir['name'], '')]
+    message('Adding test = @0@ and dep = @1@'.format(test_dir['name'], t_dep))
+    if t_dep != ['']
+      t += {'deps': t_dep}
+    endif
+

The code above creates a dependency between the module (*whose name is
same with the test*) and the test. This errors out for the 'libpq,
ssl, ldap and icu' because the type of these variables is dependency;
but test.depends can be one of the '[BuildTarget | CustomTarget |
CustomTargetIndex]' types, it can not be a dependency type.

And this can not create a link for the 'scripts, regress, isolation,
authentication, postmaster, recovery, subscription, brin, commit_ts,
gin, test_extensions, test_json_parser, test_misc, test_pg_dump,
typcache, unsafe_tests, worker_spi, kerberos and ecpg' tests. I think
only 'regress, isolation, test_json_parser, worker_spi and ecpg' are
wrong in this list as their modules names are not the same with their
tests.

-- 
Regards,
Nazir Bilal Yavuz
Microsoft