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]https://mesonbuild.com/Reference-manual_functions.html#test. As a result each test uses its own
"portlock" directory [1]https://github.com/postgres/postgres/blob/7202d72787d3b93b692feae62ee963238580c877/src/test/perl/PostgreSQL/Test/Cluster.pm#L172. 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
https://github.com/postgres/postgres/blob/7202d72787d3b93b692feae62ee963238580c877/src/test/perl/PostgreSQL/Test/Cluster.pm#L172
Attachments:
testwrap.patchtext/x-diff; name=testwrap.patchDownload
diff --git a/src/tools/testwrap b/src/tools/testwrap
index 8ae8fb79ba7..21c489a5beb 100755
--- a/src/tools/testwrap
+++ b/src/tools/testwrap
@@ -39,6 +39,7 @@ os.chdir(args.srcdir)
open(os.path.join(testdir, 'test.start'), 'x')
env_dict = {**os.environ,
+ 'MESON_BUILD_ROOT': args.basedir,
'TESTDATADIR': os.path.join(testdir, 'data'),
'TESTLOGDIR': os.path.join(testdir, 'log')}
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
Attachments:
meson-portlock.difftext/x-diff; charset=us-asciiDownload
diff --git c/meson.build i/meson.build
index cdf55957629..bed36153c7b 100644
--- c/meson.build
+++ i/meson.build
@@ -3414,6 +3414,8 @@ test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template'
test_env.set('PG_REGRESS', pg_regress.full_path())
test_env.set('REGRESS_SHLIB', regress_module.full_path())
test_env.set('INITDB_TEMPLATE', test_initdb_template)
+# for Cluster.pm's portlock logic
+test_env.set('top_builddir', meson.build_root())
# Add the temporary installation to the library search path on platforms where
# that works (everything but windows, basically). On windows everything
diff --git c/src/test/perl/PostgreSQL/Test/Cluster.pm i/src/test/perl/PostgreSQL/Test/Cluster.pm
index f31af70edb6..01ddaec4502 100644
--- c/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ i/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -169,8 +169,7 @@ INIT
# Otherwise, try to use a directory at the top of the build tree
# or as a last resort use the tmp_check directory
my $build_dir =
- $ENV{MESON_BUILD_ROOT}
- || $ENV{top_builddir}
+ $ENV{top_builddir}
|| $PostgreSQL::Test::Utils::tmp_check;
$portdir ||= "$build_dir/portlock";
$portdir =~ s!\\!/!g;
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