public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andreas Karlsson <[email protected]>
To: Rustam ALLAKOV <[email protected]>
To: [email protected]
To: Andres Freund <[email protected]>
Subject: Re: Add support for EXTRA_REGRESS_OPTS for meson
Date: Wed, 31 Dec 2025 02:01:05 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CALDaNm1G3qSNgVA2543BmB+dimT7FLGYksY5iLBmcLe8Rj-1dg@mail.gmail.com>
	<[email protected]>

On 3/19/25 11:25 PM, Rustam ALLAKOV wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world:  not tested
> Implements feature:       tested, failed
> Spec compliant:           tested, failed
> Documentation:            tested, failed
> 
> Hello everyone,
> for v2 patch I suggest to refactor from
> 
>> -sp = subprocess.Popen(args.test_command, env=env_dict, stdout=subprocess.PIPE)
>> +if args.testname in ['regress', 'isolation', 'ecpg'] and 'EXTRA_REGRESS_OPTS' in env_dict:
>> +    test_command = args.test_command + shlex.split(env_dict['EXTRA_REGRESS_OPTS'])
>> +else:
>> +    test_command = args.test_command
>> +
>> +sp = subprocess.Popen(test_command, env=env_dict, stdout=subprocess.PIPE)
>   
> to
> 
> -sp = subprocess.Popen(args.test_command, env=env_dict, stdout=subprocess.PIPE)
> +if args.testname in ['regress', 'isolation', 'ecpg']:
> +    test_command = args.test_command[:]
> +    if 'TEMP_CONFIG' in env_dict:
> +        test_command.insert(1, '--temp-config=' + env_dict['TEMP_CONFIG'])
> +    if 'EXTRA_REGRESS_OPTS' in env_dict:
> +        test_command += shlex.split(env_dict['EXTRA_REGRESS_OPTS'])
> +else:
> +    test_command = args.test_command
> +
> +sp = subprocess.Popen(test_command, env=env_dict, stdout=subprocess.PIPE)

Since commit 51da766494dcc84b6f8d793ecaa064363a9243c2 it is possible 
that we no longer need to use .insert() to make sure the code works on 
Windows but I need to think a bit more on it.

But added support for TEMP_CONFIG.

> I double checked whether shlex module was built in Python, and yes it is,
> so no need for additional requirement.txt input for pip to install.

Thanks!

> in addition to the above, might be worth to add some documentation like
> 
> Environment Variables Supported:
> EXTRA_REGRESS_OPTS: Additional options to pass to regression, isolation, or ecpg tests.
> TEMP_CONFIG: Specify a temporary configuration file for testing purposes.
> Example Usage:
> # Use EXTRA_REGRESS_OPTS to load an extension
>    EXTRA_REGRESS_OPTS="--load-extension=pgcrypto" meson test
> # Use TEMP_CONFIG to specify a temporary configuration file
>    TEMP_CONFIG="path/to/test.conf" meson test
> # Use both EXTRA_REGRESS_OPTS and TEMP_CONFIG together
>    TEMP_CONFIG="path/to/test.conf" EXTRA_REGRESS_OPTS="--load-extension=pgcrypto" meson test

Yeah, we probably should. But not sure where, maybe at 
https://www.postgresql.org/docs/current/install-meson.html or did you 
imagine somewhere else?

> Should we cover these new lines with test? Asking, because I see EXTRA_REGRESS_OPTS
> being tested for autotools in src/interfaces/ecpg/test/makefile

As far as I can see they are not tested there, but maybe we should test 
them.

Attached version 2 of the patch.

Andreas


Attachments:

  [text/x-patch] v2-0002-meson-Add-support-for-EXTRA_REGRESS_OPTS-and-TEMP.patch (1.3K, 2-v2-0002-meson-Add-support-for-EXTRA_REGRESS_OPTS-and-TEMP.patch)
  download | inline diff:
From 0e73ab29af1ccab9c4dd7453f07b41a4527b0340 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email protected]>
Date: Wed, 31 Dec 2025 01:48:56 +0100
Subject: [PATCH v2 2/2] meson: Add support for EXTRA_REGRESS_OPTS and
 TEMP_CONFIG

Add support for the EXTRA_REGRESS_OPTS and TEMP_CONFIG environment
variables in our Meson build which work just like with make and
apply to all regress, ecpg and isolation tests.
---
 src/tools/testwrap | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/tools/testwrap b/src/tools/testwrap
index e91296ecd15..165d35fba98 100755
--- a/src/tools/testwrap
+++ b/src/tools/testwrap
@@ -4,6 +4,7 @@ import argparse
 import shutil
 import subprocess
 import os
+import shlex
 import sys
 
 parser = argparse.ArgumentParser()
@@ -53,6 +54,14 @@ env_dict = {**os.environ,
 if "PG_TEST_EXTRA" not in env_dict and args.pg_test_extra:
     env_dict["PG_TEST_EXTRA"] = args.pg_test_extra
 
+# Add extra regress arguments before we add non-option arguments
+if args.testname in ['regress', 'isolation', 'ecpg']:
+    if 'TEMP_CONFIG' in env_dict:
+        args.test_command += ['--temp-config=' + env_dict['TEMP_CONFIG']]
+
+    if 'EXTRA_REGRESS_OPTS' in env_dict:
+        args.test_command += shlex.split(env_dict['EXTRA_REGRESS_OPTS'])
+
 if "TESTS" in env_dict:
     args.test_command += env_dict["TESTS"].split()
 else:
-- 
2.47.3



view thread (9+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add support for EXTRA_REGRESS_OPTS for meson
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox