meson and check-tests

On Fri May 31, 2024 at 12:02 PM CDT, Ashutosh Bapat wrote:

Hi Tristan,
Using make I can run only selected tests under src/test/regress using
TESTS=... make check-tests. I didn't find any way to do that with meson.
meson test --suite regress runs all the regression tests.

We talked this off-list at the conference. It seems we have to somehow
avoid passing pg_regress --schedule argument and instead pass the list of
tests. Any idea how to do that?

I think there are 2 solutions to this.

1. Avoid passing --schedule by default, which doesn't sound like a great
solution.

2. Teach pg_regress to ignore the --schedule option if specific tests
are passed instead.

3. Add a --no-schedule option to pg_regress which would override the
previously added --schedule option.

I personally prefer 2 or 3.

2: meson test -C build regress/regress --test-args my_specific_test
3: meson test -C build regress/regress --test-args "--no-schedule my_specific_test"

Does anyone have an opinion?

--
Tristan Partin
https://tristan.partin.io

On Sun, Jun 2, 2024 at 4:48 AM Tristan Partin <tristan@partin.io> wrote:

On Fri May 31, 2024 at 12:02 PM CDT, Ashutosh Bapat wrote:

Hi Tristan,
Using make I can run only selected tests under src/test/regress using
TESTS=... make check-tests. I didn't find any way to do that with meson.
meson test --suite regress runs all the regression tests.

We talked this off-list at the conference. It seems we have to somehow
avoid passing pg_regress --schedule argument and instead pass the list of
tests. Any idea how to do that?

I think there are 2 solutions to this.

1. Avoid passing --schedule by default, which doesn't sound like a great
solution.

2. Teach pg_regress to ignore the --schedule option if specific tests
are passed instead.

3. Add a --no-schedule option to pg_regress which would override the
previously added --schedule option.

I personally prefer 2 or 3.

2: meson test -C build regress/regress --test-args my_specific_test
3: meson test -C build regress/regress --test-args "--no-schedule my_specific_test"

Does anyone have an opinion?

if each individual sql file can be tested separately, will
`test: test_setup`?
be aslo executed as a prerequisite?

in
# ----------
# The first group of parallel tests
# ----------
test: boolean char name varchar text int2 int4 int8 oid float4 float8
bit numeric txid uuid enum money rangetypes pg_lsn regproc

# ----------
# The second group of parallel tests
# multirangetypes depends on rangetypes
# multirangetypes shouldn't run concurrently with type_sanity
# ----------
test: strings md5 numerology point lseg line box path polygon circle
date time timetz timestamp timestamptz interval inet macaddr macaddr8
multirangetypes

If we can test each individual sql file via meson, that would be great.
but based on the above comments, we need to preserve the specified order.
like:
TEST=rangetypes, multirangetypes

will first run rangetypes then multirangetypes.

can we tag/name each src/test/regress/parallel_schedule groups
like:
group1:test: boolean char name varchar text int2 int4 int8 oid float4
float8 bit numeric txid uuid enum money rangetypes pg_lsn regproc
group2:test: strings md5 numerology point lseg line box path polygon
circle date time timetz timestamp timestamptz interval inet macaddr
macaddr8 multirangetypes

Then, we can test each individual group.
TEST=group1, group2.

"Tristan Partin" <tristan@partin.io> writes:

On Fri May 31, 2024 at 12:02 PM CDT, Ashutosh Bapat wrote:

We talked this off-list at the conference. It seems we have to somehow
avoid passing pg_regress --schedule argument and instead pass the list of
tests. Any idea how to do that?

I think there are 2 solutions to this.
1. Avoid passing --schedule by default, which doesn't sound like a great
solution.
2. Teach pg_regress to ignore the --schedule option if specific tests
are passed instead.
3. Add a --no-schedule option to pg_regress which would override the
previously added --schedule option.
I personally prefer 2 or 3.

Just to refresh peoples' memory of what the Makefiles do:
src/test/regress/GNUmakefile has

check: all
$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)

check-tests: all | temp-install
$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS)

(and parallel cases for installcheck etc). AFAICS, meson.build has
no equivalent to the EXTRA_TESTS add-on, nor does it have behavior
equivalent to check-tests' substitution of $(TESTS) for --schedule.
But I suggest that those behaviors have stood for a long time and
so the appropriate thing to do is duplicate them as best we can,
not invent something different.

regards, tom lane

On 2024-06-02 Su 01:25, Tom Lane wrote:

"Tristan Partin" <tristan@partin.io> writes:

On Fri May 31, 2024 at 12:02 PM CDT, Ashutosh Bapat wrote:

We talked this off-list at the conference. It seems we have to somehow
avoid passing pg_regress --schedule argument and instead pass the list of
tests. Any idea how to do that?

I think there are 2 solutions to this.
1. Avoid passing --schedule by default, which doesn't sound like a great
solution.
2. Teach pg_regress to ignore the --schedule option if specific tests
are passed instead.
3. Add a --no-schedule option to pg_regress which would override the
previously added --schedule option.
I personally prefer 2 or 3.

Just to refresh peoples' memory of what the Makefiles do:
src/test/regress/GNUmakefile has

check: all
$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)

check-tests: all | temp-install
$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS)

(and parallel cases for installcheck etc). AFAICS, meson.build has
no equivalent to the EXTRA_TESTS add-on, nor does it have behavior
equivalent to check-tests' substitution of $(TESTS) for --schedule.
But I suggest that those behaviors have stood for a long time and
so the appropriate thing to do is duplicate them as best we can,
not invent something different.

+1

cheers

andrew

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

Hi,

On Sun, 2 Jun 2024 at 07:06, jian he <jian.universality@gmail.com> wrote:

On Sun, Jun 2, 2024 at 4:48 AM Tristan Partin <tristan@partin.io> wrote:

On Fri May 31, 2024 at 12:02 PM CDT, Ashutosh Bapat wrote:

Hi Tristan,
Using make I can run only selected tests under src/test/regress using
TESTS=... make check-tests. I didn't find any way to do that with meson.
meson test --suite regress runs all the regression tests.

We talked this off-list at the conference. It seems we have to somehow
avoid passing pg_regress --schedule argument and instead pass the list of
tests. Any idea how to do that?

I think there are 2 solutions to this.

1. Avoid passing --schedule by default, which doesn't sound like a great
solution.

2. Teach pg_regress to ignore the --schedule option if specific tests
are passed instead.

3. Add a --no-schedule option to pg_regress which would override the
previously added --schedule option.

I personally prefer 2 or 3.

2: meson test -C build regress/regress --test-args my_specific_test
3: meson test -C build regress/regress --test-args "--no-schedule my_specific_test"

Does anyone have an opinion?

if each individual sql file can be tested separately, will
`test: test_setup`?
be aslo executed as a prerequisite?

Yes, it is required to run at least two setup tests because regress
tests require a database to be created:

1- The initdb executable is needed, and it is installed by the
'tmp_install' test for the tests.

2- Although the initdb executable exists, it is not enough by itself.
Regress tests copies its database contents from the template
directory, instead of running initdb for each test [1]252dcb3239 (This is the
default behaviour in the meson builds' regress tests). This template
directory is created by the 'initdb_cache' test.

[1]: 252dcb3239

--
Regards,
Nazir Bilal Yavuz
Microsoft

On Sun Jun 2, 2024 at 12:25 AM CDT, Tom Lane wrote:

"Tristan Partin" <tristan@partin.io> writes:

On Fri May 31, 2024 at 12:02 PM CDT, Ashutosh Bapat wrote:

We talked this off-list at the conference. It seems we have to somehow
avoid passing pg_regress --schedule argument and instead pass the list of
tests. Any idea how to do that?

I think there are 2 solutions to this.
1. Avoid passing --schedule by default, which doesn't sound like a great
solution.
2. Teach pg_regress to ignore the --schedule option if specific tests
are passed instead.
3. Add a --no-schedule option to pg_regress which would override the
previously added --schedule option.
I personally prefer 2 or 3.

Just to refresh peoples' memory of what the Makefiles do:
src/test/regress/GNUmakefile has

check: all
$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)

check-tests: all | temp-install
$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS)

(and parallel cases for installcheck etc). AFAICS, meson.build has
no equivalent to the EXTRA_TESTS add-on, nor does it have behavior
equivalent to check-tests' substitution of $(TESTS) for --schedule.
But I suggest that those behaviors have stood for a long time and
so the appropriate thing to do is duplicate them as best we can,
not invent something different.

In theory, this makes sense. In practice, this is hard to emulate. We
could make the check-tests a Meson run_target() instead of another
test(), which would end up running the same tests more than once.

We could also add the same functionality proposed in my email to the
Makefile, so they mirror each other. check-tests would then just become
a legacy target name.

--
Tristan Partin
https://tristan.partin.io

On Mon, Jun 3, 2024 at 10:04 PM Tristan Partin <tristan@partin.io> wrote:

On Sun Jun 2, 2024 at 12:25 AM CDT, Tom Lane wrote:

"Tristan Partin" <tristan@partin.io> writes:

On Fri May 31, 2024 at 12:02 PM CDT, Ashutosh Bapat wrote:

We talked this off-list at the conference. It seems we have to somehow
avoid passing pg_regress --schedule argument and instead pass the

list of

tests. Any idea how to do that?

I think there are 2 solutions to this.
1. Avoid passing --schedule by default, which doesn't sound like a

great

solution.
2. Teach pg_regress to ignore the --schedule option if specific tests
are passed instead.
3. Add a --no-schedule option to pg_regress which would override the
previously added --schedule option.
I personally prefer 2 or 3.

Just to refresh peoples' memory of what the Makefiles do:
src/test/regress/GNUmakefile has

check: all
$(pg_regress_check) $(REGRESS_OPTS)

--schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)

check-tests: all | temp-install
$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS)

$(EXTRA_TESTS)

(and parallel cases for installcheck etc). AFAICS, meson.build has
no equivalent to the EXTRA_TESTS add-on, nor does it have behavior
equivalent to check-tests' substitution of $(TESTS) for --schedule.
But I suggest that those behaviors have stood for a long time and
so the appropriate thing to do is duplicate them as best we can,
not invent something different.

In theory, this makes sense. In practice, this is hard to emulate. We
could make the check-tests a Meson run_target() instead of another
test(), which would end up running the same tests more than once.

meson has changed the way we run individual perl tests and that looks
better. So I am fine if meson provides a better way to do what `make
check-tests` does. But changing pg_regress seems a wrong choice or even
making changes to the current make system. Instead we should make meson
pass the right arguments to pg_regress. In this case, it should not pass
--schedule when we need `make check-tests` like functionality.

Just adding check-tests as new target won't help we need some way to
specify "which tests" to run. Thus by default this target should not run
any tests? I don't understand meson well. So I might be completely wrong?

How about the following options?
1. TESTS="..." meson test --suite regress - would run the specified tests
from regress

2. Make `meson test --suite regress / regress/partition_join` run
partition_join.sql test. I am not how to specify multiple tests in this
command. May be `meson test --suite regress /
regress/test_setup,partition_join` will do that. make check-tests allows
one to run multiple tests like TESTS="test_setup partition_join" make
check-tests.

--
Best Wishes,
Ashutosh Bapat

On Wed, Jun 5, 2024 at 7:26 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:

On Mon, Jun 3, 2024 at 10:04 PM Tristan Partin <tristan@partin.io> wrote:

On Sun Jun 2, 2024 at 12:25 AM CDT, Tom Lane wrote:

"Tristan Partin" <tristan@partin.io> writes:

On Fri May 31, 2024 at 12:02 PM CDT, Ashutosh Bapat wrote:

We talked this off-list at the conference. It seems we have to somehow
avoid passing pg_regress --schedule argument and instead pass the list of
tests. Any idea how to do that?

I think there are 2 solutions to this.
1. Avoid passing --schedule by default, which doesn't sound like a great
solution.
2. Teach pg_regress to ignore the --schedule option if specific tests
are passed instead.
3. Add a --no-schedule option to pg_regress which would override the
previously added --schedule option.
I personally prefer 2 or 3.

Just to refresh peoples' memory of what the Makefiles do:
src/test/regress/GNUmakefile has

check: all
$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)

check-tests: all | temp-install
$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS)

(and parallel cases for installcheck etc). AFAICS, meson.build has
no equivalent to the EXTRA_TESTS add-on, nor does it have behavior
equivalent to check-tests' substitution of $(TESTS) for --schedule.
But I suggest that those behaviors have stood for a long time and
so the appropriate thing to do is duplicate them as best we can,
not invent something different.

In theory, this makes sense. In practice, this is hard to emulate. We
could make the check-tests a Meson run_target() instead of another
test(), which would end up running the same tests more than once.

meson has changed the way we run individual perl tests and that looks better. So I am fine if meson provides a better way to do what `make check-tests` does. But changing pg_regress seems a wrong choice or even making changes to the current make system. Instead we should make meson pass the right arguments to pg_regress. In this case, it should not pass --schedule when we need `make check-tests` like functionality.

Just adding check-tests as new target won't help we need some way to specify "which tests" to run. Thus by default this target should not run any tests? I don't understand meson well. So I might be completely wrong?

How about the following options?
1. TESTS="..." meson test --suite regress - would run the specified tests from regress

2. Make `meson test --suite regress / regress/partition_join` run partition_join.sql test. I am not how to specify multiple tests in this command. May be `meson test --suite regress / regress/test_setup,partition_join` will do that. make check-tests allows one to run multiple tests like TESTS="test_setup partition_join" make check-tests.

hi. I think it's a good feature for development.
The full regression test is still a very long time to run.

sometimes when you implement a feature, you are pretty sure some
changes will only happen in a single sql file.
so this can make it more faster.

Hi,

I’ve been working on a patch and wanted to share my approach, which
might be helpful for others. The patch removes the '--schedule' and
'${schedule_file}' options from the regress/regress test command when
the TESTS environment variable is set. Instead, it appends the TESTS
variable to the end of the command.

Please note that setup suite tests (at least tmp_install and
initdb_cache) should be executed before running these tests. One
drawback is that while the Meson logs will still show the test command
with the '--schedule' and '${schedule_file}' options, the actual
command used will be changed.

Some examples after the patched build:

$ meson test --suite regress -> fails
$ TESTS="create_table copy jsonb" meson test --suite regress -> fails
### run required setup suite tests
$ meson test tmp_install
$ meson test initdb_cache
###
$ meson test --suite regress -> passes (12s)
$ TESTS="copy" meson test --suite regress -> passes (0.35s)
$ TESTS="copy jsonb" meson test --suite regress -> passes (0.52s)
$ TESTS='select_into' meson test --suite regress -> fails
$ TESTS='test_setup select_into' meson test --suite regress -> passes (0.52s)
$ TESTS='rangetypes multirangetypes' meson test --suite regress -> fails
$ TESTS='test_setup multirangetypes rangetypes' meson test --suite
regres -> fails
$ TESTS='test_setup rangetypes multirangetypes' meson test --suite
regress -> passes (0.91s)

Any feedback would be appreciated.

--
Regards,
Nazir Bilal Yavuz
Microsoft

On Fri, Sep 20, 2024 at 6:25 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Hi,

I’ve been working on a patch and wanted to share my approach, which
might be helpful for others. The patch removes the '--schedule' and
'${schedule_file}' options from the regress/regress test command when
the TESTS environment variable is set. Instead, it appends the TESTS
variable to the end of the command.

Please note that setup suite tests (at least tmp_install and
initdb_cache) should be executed before running these tests. One
drawback is that while the Meson logs will still show the test command
with the '--schedule' and '${schedule_file}' options, the actual
command used will be changed.

Some examples after the patched build:

$ meson test --suite regress -> fails
$ TESTS="create_table copy jsonb" meson test --suite regress -> fails
### run required setup suite tests
$ meson test tmp_install
$ meson test initdb_cache
###
$ meson test --suite regress -> passes (12s)
$ TESTS="copy" meson test --suite regress -> passes (0.35s)
$ TESTS="copy jsonb" meson test --suite regress -> passes (0.52s)
$ TESTS='select_into' meson test --suite regress -> fails
$ TESTS='test_setup select_into' meson test --suite regress -> passes (0.52s)
$ TESTS='rangetypes multirangetypes' meson test --suite regress -> fails
$ TESTS='test_setup multirangetypes rangetypes' meson test --suite
regres -> fails
$ TESTS='test_setup rangetypes multirangetypes' meson test --suite
regress -> passes (0.91s)

Any feedback would be appreciated.

hi. Thanks for your work!
I do find some issues.

TESTS="copy jsonb jsonb" meson test --suite regress
one will fail. not sure this is expected?

in [1]/messages/by-id/CAN55FZ3t+eDgKtsDoyi0UYwzbMkKDfqJgvsbamar9CvY_6qWPw@mail.gmail.com you mentioned "setup", but that "setup" is more or less like
"meson test --suite setup --suite regress"
but originally, I thought was about "src/test/regress/sql/test_setup.sql".
for example, now you cannot run src/test/regress/sql/stats_ext.sql
without first running test_setup.sql, because some functions (like fipshash)
live in test_setup.sql.

so
TESTS="copy jsonb stats_ext" meson test --suite regress
will fail.

to make it work we need change it to
TESTS="test_setup copy jsonb stats_ext" meson test --suite regress

Many tests depend on test_setup.sql, maybe we can implicitly prepend it.
Another dependency issue. alter_table depending on create_index.

TESTS="test_setup alter_table" meson test --suite regress
will fail.
TESTS="test_setup create_index alter_table" meson test --suite regress
will work.

[1]: /messages/by-id/CAN55FZ3t+eDgKtsDoyi0UYwzbMkKDfqJgvsbamar9CvY_6qWPw@mail.gmail.com

Hi,

On Sat, 21 Sept 2024 at 09:01, jian he <jian.universality@gmail.com> wrote:

hi. Thanks for your work!

Thank you for looking into this!

I do find some issues.

TESTS="copy jsonb jsonb" meson test --suite regress
one will fail. not sure this is expected?

Yes, that is expected.

in [1] you mentioned "setup", but that "setup" is more or less like
"meson test --suite setup --suite regress"
but originally, I thought was about "src/test/regress/sql/test_setup.sql".
for example, now you cannot run src/test/regress/sql/stats_ext.sql
without first running test_setup.sql, because some functions (like fipshash)
live in test_setup.sql.

Postgres binaries are created at the build step in the make builds so
these binaries can be used for the tests. But in the meson builds,
they are created at the 'meson test --suite setup' for the testing
('meson install' command creates binaries as well but they are not for
testing, they are for installing binaries to the OS). So, 'meson test
--suite setup' should be run before running regression tests.

so
TESTS="copy jsonb stats_ext" meson test --suite regress
will fail.

to make it work we need change it to
TESTS="test_setup copy jsonb stats_ext" meson test --suite regress

Many tests depend on test_setup.sql, maybe we can implicitly prepend it.
Another dependency issue. alter_table depending on create_index.

TESTS="test_setup alter_table" meson test --suite regress
will fail.
TESTS="test_setup create_index alter_table" meson test --suite regress
will work.

Yes, I realized that but since that is how it is done in the make
builds, I didn't want to change the behaviour. Also, I think it makes
sense to leave it to the tester. It is more flexible in that way.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Hi,

On Mon, 23 Sept 2024 at 11:46, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

On Sat, 21 Sept 2024 at 09:01, jian he <jian.universality@gmail.com> wrote:

in [1] you mentioned "setup", but that "setup" is more or less like
"meson test --suite setup --suite regress"
but originally, I thought was about "src/test/regress/sql/test_setup.sql".
for example, now you cannot run src/test/regress/sql/stats_ext.sql
without first running test_setup.sql, because some functions (like fipshash)
live in test_setup.sql.

Postgres binaries are created at the build step in the make builds so
these binaries can be used for the tests. But in the meson builds,
they are created at the 'meson test --suite setup' for the testing
('meson install' command creates binaries as well but they are not for
testing, they are for installing binaries to the OS). So, 'meson test
--suite setup' should be run before running regression tests.

The above sentence lacks some information. It appears that if binaries
are not created beforehand (only running configure, not make), they
are generated during tests in the make builds. This also applies to
meson builds when the meson test command is run, as meson executes
setup suite tests first, which creates the binaries. However, if we
specify a different test suite, like regress in this case, the setup
suite tests are not executed, and the binaries are not created,
preventing the tests from running. I am not sure how to configure
meson builds to run setup suite tests if they are not executed
beforehand.

--
Regards,
Nazir Bilal Yavuz
Microsoft

On Mon, Sep 23, 2024 at 2:16 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Hi,

On Sat, 21 Sept 2024 at 09:01, jian he <jian.universality@gmail.com> wrote:

hi. Thanks for your work!

Thank you for looking into this!

I do find some issues.

TESTS="copy jsonb jsonb" meson test --suite regress
one will fail. not sure this is expected?

Yes, that is expected.

Agreed. It's the same behaviour with `make check-tests`
TESTS="copy jsonb jsonb" make check-tests

# initializing database system by copying initdb template
# using temp instance on port 65312 with PID 880081
ok 1 - copy 51 ms
ok 2 - jsonb 143 ms
not ok 3 - jsonb 179 ms
1..3
# 1 of 3 tests failed.

so
TESTS="copy jsonb stats_ext" meson test --suite regress
will fail.

to make it work we need change it to
TESTS="test_setup copy jsonb stats_ext" meson test --suite regress

Many tests depend on test_setup.sql, maybe we can implicitly prepend it.
Another dependency issue. alter_table depending on create_index.

TESTS="test_setup alter_table" meson test --suite regress
will fail.
TESTS="test_setup create_index alter_table" meson test --suite regress
will work.

Yes, I realized that but since that is how it is done in the make
builds, I didn't want to change the behaviour. Also, I think it makes
sense to leave it to the tester. It is more flexible in that way.

Since meson has a setup suite already, it might have been a good idea
to do as Jian suggested. But a. setup is also another suite and not a
setup step per say. b. the dependencies between regression tests are
not documented well or rather we don't have a way to specify which
test depends upon which. So we can't infer the .sql files that need to
be run as a setup step. Somebody could add a dependency without meson
or make being aware of that and tests will fail again. So I think we
have to leave it as is. If we get to that point we should fix both
make as well as meson. But not as part of this exercise.

It's a bit inconvenient that we don't see whether an individual test
failed or succeeded on the screen; we need to open testlog.txt for the
same. But that's true with the regress suite generally so no
complaints there.

Individual TAP tests are run using `meson test -C <build dir>
<suite>:<test>` syntax. If we can run individual SQL tests using same
syntax e.g. `meson test regress:partition_join` that would make it
consistent with the way TAP tests are run. But we need to make sure
that the test later in the syntax would see the objects left behind by
prior tests. E.g. `meson test regress:test_setup
regress:partition_join` should see both tests passing. partition_join
uses some tables created by test_setup, so those need to be run
sequentially. Is that doable?

--
Best Wishes,
Ashutosh Bapat

Hi,

Thanks for looking into this!

On Wed, 25 Sept 2024 at 13:27, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:

On Mon, Sep 23, 2024 at 2:16 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

On Sat, 21 Sept 2024 at 09:01, jian he <jian.universality@gmail.com> wrote:

so
TESTS="copy jsonb stats_ext" meson test --suite regress
will fail.

to make it work we need change it to
TESTS="test_setup copy jsonb stats_ext" meson test --suite regress

Many tests depend on test_setup.sql, maybe we can implicitly prepend it.
Another dependency issue. alter_table depending on create_index.

TESTS="test_setup alter_table" meson test --suite regress
will fail.
TESTS="test_setup create_index alter_table" meson test --suite regress
will work.

Yes, I realized that but since that is how it is done in the make
builds, I didn't want to change the behaviour. Also, I think it makes
sense to leave it to the tester. It is more flexible in that way.

Since meson has a setup suite already, it might have been a good idea
to do as Jian suggested. But a. setup is also another suite and not a
setup step per say. b. the dependencies between regression tests are
not documented well or rather we don't have a way to specify which
test depends upon which. So we can't infer the .sql files that need to
be run as a setup step. Somebody could add a dependency without meson
or make being aware of that and tests will fail again. So I think we
have to leave it as is. If we get to that point we should fix both
make as well as meson. But not as part of this exercise.

It's a bit inconvenient that we don't see whether an individual test
failed or succeeded on the screen; we need to open testlog.txt for the
same. But that's true with the regress suite generally so no
complaints there.

Thanks for sharing your thoughts.

Individual TAP tests are run using `meson test -C <build dir>
<suite>:<test>` syntax. If we can run individual SQL tests using same
syntax e.g. `meson test regress:partition_join` that would make it
consistent with the way TAP tests are run. But we need to make sure
that the test later in the syntax would see the objects left behind by
prior tests. E.g. `meson test regress:test_setup
regress:partition_join` should see both tests passing. partition_join
uses some tables created by test_setup, so those need to be run
sequentially. Is that doable?

I think that makes sense, but it is not easily achievable right now.
The difference between TAP tests and regress/regress tests is that TAP
tests are registered individually, whereas regress/regress tests are
registered as one (with the --schedule option). This means we need to
register these tests one by one (instead of passing them with the
--schedule option) to the Meson build system in order to run them as
'meson test <test_group>:<test>'.

Additionally, the patch I shared earlier was only for regress/regress
tests. From what I understand from here [1]/messages/by-id/1364.1717305911@sss.pgh.pa.us, only regress/regress
tests support 'make check-tests', so the patch seems correct. I
experimented with how we can implement something similar for other
types of tests, including other regression, isolation, and ECPG tests.
The attached patch works for all types of tests but only for the Meson
builds. For example you can run:

$ meson test --suite setup
$ TESTS='check check_btree' meson test amcheck/regress
$ TESTS='ddl stream' meson test test_decoding/regress
$ TESTS='oldest_xmin skip_snapshot_restore' meson test test_decoding/isolation
$ TESTS='sql/prepareas compat_informix/dec_test' meson test ecpg/ecpg

What do you think about that behaviour? It is different from 'make
check-tests' but it looks useful to me.

[1]: /messages/by-id/1364.1717305911@sss.pgh.pa.us

--
Regards,
Nazir Bilal Yavuz
Microsoft

On Wed, Sep 25, 2024 at 8:24 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Hi,

Thanks for looking into this!

On Wed, 25 Sept 2024 at 13:27, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:

On Mon, Sep 23, 2024 at 2:16 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

On Sat, 21 Sept 2024 at 09:01, jian he <jian.universality@gmail.com> wrote:

so
TESTS="copy jsonb stats_ext" meson test --suite regress
will fail.

to make it work we need change it to
TESTS="test_setup copy jsonb stats_ext" meson test --suite regress

Many tests depend on test_setup.sql, maybe we can implicitly prepend it.
Another dependency issue. alter_table depending on create_index.

TESTS="test_setup alter_table" meson test --suite regress
will fail.
TESTS="test_setup create_index alter_table" meson test --suite regress
will work.

Yes, I realized that but since that is how it is done in the make
builds, I didn't want to change the behaviour. Also, I think it makes
sense to leave it to the tester. It is more flexible in that way.

Since meson has a setup suite already, it might have been a good idea
to do as Jian suggested. But a. setup is also another suite and not a
setup step per say. b. the dependencies between regression tests are
not documented well or rather we don't have a way to specify which
test depends upon which. So we can't infer the .sql files that need to
be run as a setup step. Somebody could add a dependency without meson
or make being aware of that and tests will fail again. So I think we
have to leave it as is. If we get to that point we should fix both
make as well as meson. But not as part of this exercise.

It's a bit inconvenient that we don't see whether an individual test
failed or succeeded on the screen; we need to open testlog.txt for the
same. But that's true with the regress suite generally so no
complaints there.

Thanks for sharing your thoughts.

Individual TAP tests are run using `meson test -C <build dir>
<suite>:<test>` syntax. If we can run individual SQL tests using same
syntax e.g. `meson test regress:partition_join` that would make it
consistent with the way TAP tests are run. But we need to make sure
that the test later in the syntax would see the objects left behind by
prior tests. E.g. `meson test regress:test_setup
regress:partition_join` should see both tests passing. partition_join
uses some tables created by test_setup, so those need to be run
sequentially. Is that doable?

I think that makes sense, but it is not easily achievable right now.
The difference between TAP tests and regress/regress tests is that TAP
tests are registered individually, whereas regress/regress tests are
registered as one (with the --schedule option). This means we need to
register these tests one by one (instead of passing them with the
--schedule option) to the Meson build system in order to run them as
'meson test <test_group>:<test>'.

I understand. Probably that's a shortcoming in the way our meson
support is designed. I don't see a way to fix it without changing a
lot. So I guess the current interface is good enough.

Additionally, the patch I shared earlier was only for regress/regress
tests. From what I understand from here [1], only regress/regress
tests support 'make check-tests', so the patch seems correct. I
experimented with how we can implement something similar for other
types of tests, including other regression, isolation, and ECPG tests.
The attached patch works for all types of tests but only for the Meson
builds. For example you can run:

$ meson test --suite setup
$ TESTS='check check_btree' meson test amcheck/regress
$ TESTS='ddl stream' meson test test_decoding/regress
$ TESTS='oldest_xmin skip_snapshot_restore' meson test test_decoding/isolation
$ TESTS='sql/prepareas compat_informix/dec_test' meson test ecpg/ecpg

What do you think about that behaviour? It is different from 'make
check-tests' but it looks useful to me.

I think that would be a good enhancement, if a particular regression
set takes longer e.g. the one in test_decoding takes a few seconds.
When we worked on PG_TEST_EXTRA, it was advised to keep feature parity
between meson and make. I guess, a similar advice applies here as well
and we will have to change make to support these options. But that
will be more work.

Let's split the patch into two 1. supporting TESTS in meson only for
regress/regress, 2. extending that support to other suites. The first
patch will bring meson inline with make as far as running a subset of
regression tests is concerned and can be committed separately. We will
seek opinions on the second patch and commit it separately if it takes
time. It will be good to see the support for running a subset of
regression in meson ASAP so that developers can save time running
entire regression suite when not needed. The second one will be an
additional feature that can wait if it takes more time to add it to
both meson and make.

--
Best Wishes,
Ashutosh Bapat

Hi,

On Thu, 26 Sept 2024 at 08:45, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:

On Wed, Sep 25, 2024 at 8:24 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Additionally, the patch I shared earlier was only for regress/regress
tests. From what I understand from here [1], only regress/regress
tests support 'make check-tests', so the patch seems correct. I
experimented with how we can implement something similar for other
types of tests, including other regression, isolation, and ECPG tests.
The attached patch works for all types of tests but only for the Meson
builds. For example you can run:

$ meson test --suite setup
$ TESTS='check check_btree' meson test amcheck/regress
$ TESTS='ddl stream' meson test test_decoding/regress
$ TESTS='oldest_xmin skip_snapshot_restore' meson test test_decoding/isolation
$ TESTS='sql/prepareas compat_informix/dec_test' meson test ecpg/ecpg

What do you think about that behaviour? It is different from 'make
check-tests' but it looks useful to me.

I think that would be a good enhancement, if a particular regression
set takes longer e.g. the one in test_decoding takes a few seconds.
When we worked on PG_TEST_EXTRA, it was advised to keep feature parity
between meson and make. I guess, a similar advice applies here as well
and we will have to change make to support these options. But that
will be more work.

Let's split the patch into two 1. supporting TESTS in meson only for
regress/regress, 2. extending that support to other suites. The first
patch will bring meson inline with make as far as running a subset of
regression tests is concerned and can be committed separately. We will
seek opinions on the second patch and commit it separately if it takes
time. It will be good to see the support for running a subset of
regression in meson ASAP so that developers can save time running
entire regression suite when not needed. The second one will be an
additional feature that can wait if it takes more time to add it to
both meson and make.

I agree with you. I splitted the patch into two like you said.

--
Regards,
Nazir Bilal Yavuz
Microsoft

v3, 0001 documentation:
We can at least write something on
https://wiki.postgresql.org/wiki/Meson about this feature.

TESTS='check check_btree' meson test amcheck/regress --verbose
works, but I feel like there is a discoverability issue .

TESTS='check check_btree' meson test amcheck/regress --list --verbose
shows:
postgresql:amcheck / amcheck/regress

For small tests, listing all the available tests would be great.

On Fri, Oct 4, 2024 at 6:43 PM jian he <jian.universality@gmail.com> wrote:

v3, 0001 documentation:
We can at least write something on
https://wiki.postgresql.org/wiki/Meson about this feature.

TESTS and make check-tests are both not documented under make. I don't
know the reason. IMO we don't have to document it in meson. But if we
want to document it in meson, we should document it in make as well.

--
Best Wishes,
Ashutosh Bapat

Hi Nazir,

On Thu, Sep 26, 2024 at 4:14 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Let's split the patch into two 1. supporting TESTS in meson only for
regress/regress, 2. extending that support to other suites. The first
patch will bring meson inline with make as far as running a subset of
regression tests is concerned and can be committed separately. We will
seek opinions on the second patch and commit it separately if it takes
time. It will be good to see the support for running a subset of
regression in meson ASAP so that developers can save time running
entire regression suite when not needed. The second one will be an
additional feature that can wait if it takes more time to add it to
both meson and make.

I agree with you. I splitted the patch into two like you said.

Sorry for delay in reply.

--schedule or the test selection becomes part of the test command
itself in the current master. By passing it as an argument, in these
patches, we are allowing those to be overridden if TESTS is set at the
time of running the test. I like this idea. I was pondering whether we
really need two separate arguments --schedule and --tests
respectively. IIUC, we can't pass them as a single argument (say
--test-selection or --selection) because the subsequent --schedule
would be treated as a separate argument if not quoted correctly. That
isn't a problem with value of tests. To avoid quoting '--schedule
...', we have two separate arguments. Am I right?

It might be better to make this explicit in the code -- by making sure
that only one of them is passed and writing a comment about it.
ArgumentParser might have some trick to specify that passing both the
arguments is an error.

Also I don't think "Note that setup
# suite tests (at least tmp_install and initdb_cache tests) may need to be run
# before running these tests." is needed. This is true irrespective of
whether we specify TESTS or not.

--
Best Wishes,
Ashutosh Bapat