Обсуждение: TAP test started using meson, can get a tcp port already used by another test.

Hello,

We running TAP tests which operates with a lot of instances. And some of 
these tests randomly fail due "address already in use". It turned out 
that the meson does not set environment variable MESON_BUILD_ROOT when 
running the test() function [0]. As a result each test uses its own 
"portlock" directory [1]. The small TAP test 001_portlock_env_test.pl 
shows the test environment variables. As we can see MESON_BUILD_ROOT is 
really undefined.

Can we explicitly set the MESON_BUILD_ROOT environment variable when 
running a test? With included patch for the src/tools/testwrap file, 
each instance gets an unique TCP port.

Thanks!

Best regards, Roman Zharkov

[0] https://mesonbuild.com/Reference-manual_functions.html#test
[1] 

https://github.com/postgres/postgres/blob/7202d72787d3b93b692feae62ee963238580c877/src/test/perl/PostgreSQL/Test/Cluster.pm#L172

On 2025-02-21 Fr 3:58 AM, Zharkov Roman wrote:
> Hello,
>
> We running TAP tests which operates with a lot of instances. And some 
> of these tests randomly fail due "address already in use". It turned 
> out that the meson does not set environment variable MESON_BUILD_ROOT 
> when running the test() function [0]. As a result each test uses its 
> own "portlock" directory [1]. The small TAP test 
> 001_portlock_env_test.pl shows the test environment variables. As we 
> can see MESON_BUILD_ROOT is really undefined.


Argh!! Meson strikes again. Apparently this is only set ... sometimes.


>
> Can we explicitly set the MESON_BUILD_ROOT environment variable when 
> running a test? With included patch for the src/tools/testwrap file, 
> each instance gets an unique TCP port.
>
>

Seems reasonable. In the meantime you can do what the buildfarm client 
does and explicitly set PG_TEST_PORT_DIR.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On 2025-02-21 16:32, Andrew Dunstan wrote:
> In the meantime you can do what the buildfarm client does and 
> explicitly set PG_TEST_PORT_DIR.

Thank you for the answer! We tried to set PG_TEST_PORT_DIR using the 
'env' option and it works fine.

Best regards, Roman Zharkov

Hi,

On 2025-02-21 15:58:45 +0700, Zharkov Roman wrote:
> We running TAP tests which operates with a lot of instances. And some of
> these tests randomly fail due "address already in use". It turned out that
> the meson does not set environment variable MESON_BUILD_ROOT when running
> the test() function [0].

I don't think meson ever set MESON_BUILD_ROOT during tests, so afaict the
portlock logic just just never worked with meson?


> As a result each test uses its own "portlock" directory [1]. The small TAP
> test 001_portlock_env_test.pl shows the test environment variables. As we
> can see MESON_BUILD_ROOT is really undefined.

I'm rather baffled this hasn't caused more problems...

I have recently seen a CI failure that looked like it likely was caused by
this. It was in a back branch, and I thought that was the explanation, due to
the portlock logic not yet existing. But the portlock logic is old enough, so
it likely was this.

But I would have expected many more errors.


> Can we explicitly set the MESON_BUILD_ROOT environment variable when running
> a test? With included patch for the src/tools/testwrap file, each instance
> gets an unique TCP port.

I don't like that, feels like a "namespace violation" to set a MESON_*
variable, why not set the top_builddir or a dedicated variable?

And I don't think it should be done in testwrap, but to test_env in the
top-level meson.build.

Something like the attached?

Greetings,

Andres Freund

On 2025-02-21 Fr 10:50 AM, Andres Freund wrote:
>
>
>> Can we explicitly set the MESON_BUILD_ROOT environment variable when running
>> a test? With included patch for the src/tools/testwrap file, each instance
>> gets an unique TCP port.
> I don't like that, feels like a "namespace violation" to set a MESON_*
> variable, why not set the top_builddir or a dedicated variable?
>
> And I don't think it should be done in testwrap, but to test_env in the
> top-level meson.build.
>
> Something like the attached?
>

LGTM


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi,

On 2025-02-21 11:04:49 -0500, Andrew Dunstan wrote:
> On 2025-02-21 Fr 10:50 AM, Andres Freund wrote:
> > > Can we explicitly set the MESON_BUILD_ROOT environment variable when running
> > > a test? With included patch for the src/tools/testwrap file, each instance
> > > gets an unique TCP port.
> > I don't like that, feels like a "namespace violation" to set a MESON_*
> > variable, why not set the top_builddir or a dedicated variable?
> > 
> > And I don't think it should be done in testwrap, but to test_env in the
> > top-level meson.build.
> > 
> > Something like the attached?
> > 
> 
> LGTM

Thanks for the quick review!

For some reason perltidy moves the $ENV{top_builddir} into the line with the =
once there's only one \n||. Somewhat annoying.

Pushed.

And thanks for the report, Roman!

Greetings,

Andres

On 2025-02-21 Fr 11:49 AM, Andres Freund wrote:
> Hi,
>
> On 2025-02-21 11:04:49 -0500, Andrew Dunstan wrote:
>> On 2025-02-21 Fr 10:50 AM, Andres Freund wrote:
>>>> Can we explicitly set the MESON_BUILD_ROOT environment variable when running
>>>> a test? With included patch for the src/tools/testwrap file, each instance
>>>> gets an unique TCP port.
>>> I don't like that, feels like a "namespace violation" to set a MESON_*
>>> variable, why not set the top_builddir or a dedicated variable?
>>>
>>> And I don't think it should be done in testwrap, but to test_env in the
>>> top-level meson.build.
>>>
>>> Something like the attached?
>>>
>> LGTM
> Thanks for the quick review!
>
> For some reason perltidy moves the $ENV{top_builddir} into the line with the =
> once there's only one \n||. Somewhat annoying.


I would probably eat the newline.


>
> Pushed.
>

Thanks



cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On 2025-02-22 04:11, Andrew Dunstan wrote:
> On 2025-02-21 Fr 11:49 AM, Andres Freund wrote:
>> Pushed.
> Thanks

Thank you very much!

Best regards, Roman Zharkov