Hi,

Thanks for looking into this!

On Wed, 25 Sept 2024 at 13:27, Ashutosh Bapat
<ashutosh.bapat....@gmail.com> wrote:
>
> On Mon, Sep 23, 2024 at 2:16 PM Nazir Bilal Yavuz <byavu...@gmail.com> wrote:
> >
> > On Sat, 21 Sept 2024 at 09:01, jian he <jian.universal...@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], 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] https://www.postgresql.org/message-id/1364.1717305911%40sss.pgh.pa.us

--
Regards,
Nazir Bilal Yavuz
Microsoft
From 002920d8c9bf10ce1fe2f3cd4f115452a92664a8 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavu...@gmail.com>
Date: Wed, 25 Sep 2024 17:31:08 +0300
Subject: [PATCH v2] Use TESTS="" in all type of tests in the Meson builds

---
 meson.build        | 12 ++++++------
 src/tools/testwrap | 13 +++++++++++++
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/meson.build b/meson.build
index 7150f85e0fb..8efc6c09da4 100644
--- a/meson.build
+++ b/meson.build
@@ -3434,11 +3434,9 @@ foreach test_dir : tests
         '--dbname', dbname,
       ] + t.get('regress_args', [])
 
-      test_selection = []
-      if t.has_key('schedule')
-        test_selection += ['--schedule', t['schedule'],]
-      endif
+      test_schedule = t.get('schedule', [])
 
+      test_selection = []
       if kind == 'isolation'
         test_selection += t.get('specs', [])
       else
@@ -3462,12 +3460,13 @@ foreach test_dir : tests
           testwrap_base,
           '--testgroup', test_group,
           '--testname', kind,
+          '--schedule', test_schedule,
+          '--tests', test_selection,
           '--',
           test_command_base,
           '--outputdir', test_output,
           '--temp-instance', test_output / 'tmp_check',
           '--port', testport.to_string(),
-          test_selection,
         ],
         suite: test_group,
         kwargs: test_kwargs,
@@ -3482,10 +3481,11 @@ foreach test_dir : tests
             testwrap_base,
             '--testgroup', test_group_running,
             '--testname', kind,
+            '--schedule', test_schedule,
+            '--tests', test_selection,
             '--',
             test_command_base,
             '--outputdir', test_output_running,
-            test_selection,
           ],
           is_parallel: t.get('runningcheck-parallel', true),
           suite: test_group_running,
diff --git a/src/tools/testwrap b/src/tools/testwrap
index 9a270beb72d..e8686b602f8 100755
--- a/src/tools/testwrap
+++ b/src/tools/testwrap
@@ -12,6 +12,8 @@ parser.add_argument('--srcdir', help='source directory of test', type=str)
 parser.add_argument('--basedir', help='base directory of test', type=str)
 parser.add_argument('--testgroup', help='test group', type=str)
 parser.add_argument('--testname', help='test name', type=str)
+parser.add_argument('--schedule', help='schedule tests', nargs='*')
+parser.add_argument('--tests', help='tests', nargs='*')
 parser.add_argument('--skip', help='skip test (with reason)', type=str)
 parser.add_argument('test_command', nargs='*')
 
@@ -41,6 +43,17 @@ env_dict = {**os.environ,
             'TESTDATADIR': os.path.join(testdir, 'data'),
             'TESTLOGDIR': os.path.join(testdir, 'log')}
 
+# If TESTS environment variable is set, only run these tests. Note that setup
+# suite tests (at least tmp_install and initdb_cache tests) may need to be run
+# before running these tests.
+if "TESTS" in env_dict:
+    args.test_command.extend(env_dict["TESTS"].split(' '))
+else:
+    if args.schedule:
+        args.test_command += ['--schedule', ' '.join(args.schedule)]
+    if args.tests:
+        args.test_command.extend(args.tests)
+
 sp = subprocess.Popen(args.test_command, env=env_dict, stdout=subprocess.PIPE)
 # Meson categorizes a passing TODO test point as bad
 # (https://github.com/mesonbuild/meson/issues/13183).  Remove the TODO
-- 
2.45.2

Reply via email to