Thread: meson and check-tests
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?
Best Wishes,
Ashutosh Bapat
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] (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 wayto do what `make check-tests` does. But changing pg_regress seems a wrong choice or even making changes to the currentmake 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 thistarget 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 multipletests in this command. May be `meson test --suite regress / regress/test_setup,partition_join` will do that. makecheck-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.
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] 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] https://www.postgresql.org/message-id/CAN55FZ3t%2BeDgKtsDoyi0UYwzbMkKDfqJgvsbamar9CvY_6qWPw%40mail.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
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
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
hi. Actually, there is a difference compared to make. make works fine with many whitespaces. but meson you can only have one whitespace. meson test -C $BUILD7 --num-processes 20 --suite setup --verbose export TESTS="test_setup boolean char" meson test -C $BUILD7 --suite regress --verbose --it will fail. TESTS="copy jsonb" meson test -C $BUILD7 --suite regress --verbose --it will fail too. seems to need rebase, otherwise v4-0001 cannot use `git apply`.
On Mon, Nov 11, 2024 at 4:14 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote: > > Hi, > > Thanks for the feedback! > > On Mon, 11 Nov 2024 at 06:34, jian he <jian.universality@gmail.com> wrote: > > > > hi. > > Actually, there is a difference compared to make. > > make works fine with many whitespaces. but meson you can only have one > > whitespace. > > > > meson test -C $BUILD7 --num-processes 20 --suite setup --verbose > > export TESTS="test_setup boolean char" > > > > meson test -C $BUILD7 --suite regress --verbose > > --it will fail. > > > > TESTS="copy jsonb" meson test -C $BUILD7 --suite regress --verbose > > --it will fail too. > > Yes, you are right. It is fixed now. > > > seems to need rebase, otherwise v4-0001 cannot use `git apply`. > > Done. also played around with v5-0002, works fine. overall, v5-0001 and v5-0002 works as i expected.
On Thu, Oct 31, 2024 at 11:28 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>
> On Fri, 4 Oct 2024 at 18:40, Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> > --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?
>
> Yes, that is exactly why we have both '--schedule' and '--tests'
> flags. Also, a comment is added to clarify this.
The comment is useful if we want to understand this change but I feel
it's confusing when reading the code. I don't think we need the
comment. The code is clearer than before as is.
>
> > 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.
>
> I did not understand why only one of them needed to be passed at a
> time. For example in ecpg tests
> (src/interfaces/ecpg/test/meson.build), both '--schedule' and
> '--tests' options are passed.
Is it because it has both schedule as well as sql?
'ecpg': {
'expecteddir': meson.current_source_dir(),
'inputdir': meson.current_build_dir(),
'schedule': ecpg_test_files,
'sql': [
'sql/twophase',
],
I see sql/twophase is not part of ecpg_schedule and it's passes
separately to testwrap.
--
Best Wishes,
Ashutosh Bapat
Hi, On Tue, 12 Nov 2024 at 18:13, jian he <jian.universality@gmail.com> wrote: > > also played around with v5-0002, works fine. > overall, v5-0001 and v5-0002 works as i expected. Thanks for checking it! -- Regards, Nazir Bilal Yavuz Microsoft
On Tue, Nov 19, 2024 at 6:43 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>
> Hi,
>
> Thanks for checking it!
>
> On Tue, 19 Nov 2024 at 15:19, Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> >
> > On Thu, Oct 31, 2024 at 11:28 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
> > >
> > > Yes, that is exactly why we have both '--schedule' and '--tests'
> > > flags. Also, a comment is added to clarify this.
> >
> > The comment is useful if we want to understand this change but I feel
> > it's confusing when reading the code. I don't think we need the
> > comment. The code is clearer than before as is.
>
> I got it, the comment is removed.
>
> > > > 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.
> > >
> > > I did not understand why only one of them needed to be passed at a
> > > time. For example in ecpg tests
> > > (src/interfaces/ecpg/test/meson.build), both '--schedule' and
> > > '--tests' options are passed.
> >
> > Is it because it has both schedule as well as sql?
> > 'ecpg': {
> > 'expecteddir': meson.current_source_dir(),
> > 'inputdir': meson.current_build_dir(),
> > 'schedule': ecpg_test_files,
> > 'sql': [
> > 'sql/twophase',
> > ],
> >
> > I see sql/twophase is not part of ecpg_schedule and it's passes
> > separately to testwrap.
>
> Yes. All the tests without schedule option are collected in the
> test_selection in the meson.build file:
>
> test_selection = []
> if kind == 'isolation'
> test_selection += t.get('specs', [])
> else
> test_selection += t.get('sql', [])
> endif
>
> And, AFAIU all the regression test suites accept both schedule option
> and tests by their names. So, it should be safe to pass both
> --schedule and --tests.
Thanks for the explanation.
For patch 2, it will be good to introduce expanded functionality to
make as well. But patch 1 is ready for the committer. So marked
accordingly.
--
Best Wishes,
Ashutosh Bapat
On Mon, Nov 25, 2024 at 6:47 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote: > > For patch 2, it will be good to introduce expanded functionality to > make as well. But patch 1 is ready for the committer. So marked > accordingly. > Didn't find a CF entry for this. Please create and update. -- Best Wishes, Ashutosh Bapat
Hi, On Mon, 25 Nov 2024 at 16:19, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote: > > On Mon, Nov 25, 2024 at 6:47 PM Ashutosh Bapat > <ashutosh.bapat.oss@gmail.com> wrote: > > > > > For patch 2, it will be good to introduce expanded functionality to > > make as well. But patch 1 is ready for the committer. So marked > > accordingly. > > > > Didn't find a CF entry for this. Please create and update. It is added and marked as ready for the committer [1]. There are multiple 'Jian He' users in the commitfest app, so I couldn't add you as a reviewer; please add yourself. [1] https://commitfest.postgresql.org/51/5405/ -- Regards, Nazir Bilal Yavuz Microsoft