Content-Type: multipart/mixed; boundary="------------ZBKuT4gM8S92OEYypzmgLIf7"
Message-ID: <1d39c711-98cb-460f-ac44-1ab0007daeb2@proxel.se>
Date: Wed, 31 Dec 2025 02:01:05 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Add support for EXTRA_REGRESS_OPTS for meson
To: Rustam ALLAKOV <rustamallakov@gmail.com>,
 pgsql-hackers@lists.postgresql.org, Andres Freund <andres@anarazel.de>
References: <fce84be6-3158-4b9c-9b40-a403c49d2966@proxel.se>
 <CALDaNm1G3qSNgVA2543BmB+dimT7FLGYksY5iLBmcLe8Rj-1dg@mail.gmail.com>
 <174242314653.510.8908680181703298899.pgcf@coridan.postgresql.org>
From: Andreas Karlsson <andreas@proxel.se>
Content-Language: en-US
In-Reply-To: 
 <174242314653.510.8908680181703298899.pgcf@coridan.postgresql.org>
Archived-At: 
 <https://www.postgresql.org/message-id/1d39c711-98cb-460f-ac44-1ab0007daeb2%40proxel.se>
Precedence: bulk

This is a multi-part message in MIME format.
--------------ZBKuT4gM8S92OEYypzmgLIf7
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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

--------------ZBKuT4gM8S92OEYypzmgLIf7
Content-Type: text/x-patch; charset=UTF-8;
 name="v2-0002-meson-Add-support-for-EXTRA_REGRESS_OPTS-and-TEMP.patch"
Content-Disposition: attachment;
 filename*0="v2-0002-meson-Add-support-for-EXTRA_REGRESS_OPTS-and-TEMP.pa";
 filename*1="tch"
Content-Transfer-Encoding: base64

RnJvbSAwZTczYWIyOWFmMWNjYWI5YzRkZDc0NTNmMDdiNDFhNDUyN2IwMzQwIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBBbmRyZWFzIEthcmxzc29uIDxhbmRyZWFzQHByb3hl
bC5zZT4KRGF0ZTogV2VkLCAzMSBEZWMgMjAyNSAwMTo0ODo1NiArMDEwMApTdWJqZWN0OiBb
UEFUQ0ggdjIgMi8yXSBtZXNvbjogQWRkIHN1cHBvcnQgZm9yIEVYVFJBX1JFR1JFU1NfT1BU
UyBhbmQKIFRFTVBfQ09ORklHCgpBZGQgc3VwcG9ydCBmb3IgdGhlIEVYVFJBX1JFR1JFU1Nf
T1BUUyBhbmQgVEVNUF9DT05GSUcgZW52aXJvbm1lbnQKdmFyaWFibGVzIGluIG91ciBNZXNv
biBidWlsZCB3aGljaCB3b3JrIGp1c3QgbGlrZSB3aXRoIG1ha2UgYW5kCmFwcGx5IHRvIGFs
bCByZWdyZXNzLCBlY3BnIGFuZCBpc29sYXRpb24gdGVzdHMuCi0tLQogc3JjL3Rvb2xzL3Rl
c3R3cmFwIHwgOSArKysrKysrKysKIDEgZmlsZSBjaGFuZ2VkLCA5IGluc2VydGlvbnMoKykK
CmRpZmYgLS1naXQgYS9zcmMvdG9vbHMvdGVzdHdyYXAgYi9zcmMvdG9vbHMvdGVzdHdyYXAK
aW5kZXggZTkxMjk2ZWNkMTUuLjE2NWQzNWZiYTk4IDEwMDc1NQotLS0gYS9zcmMvdG9vbHMv
dGVzdHdyYXAKKysrIGIvc3JjL3Rvb2xzL3Rlc3R3cmFwCkBAIC00LDYgKzQsNyBAQCBpbXBv
cnQgYXJncGFyc2UKIGltcG9ydCBzaHV0aWwKIGltcG9ydCBzdWJwcm9jZXNzCiBpbXBvcnQg
b3MKK2ltcG9ydCBzaGxleAogaW1wb3J0IHN5cwogCiBwYXJzZXIgPSBhcmdwYXJzZS5Bcmd1
bWVudFBhcnNlcigpCkBAIC01Myw2ICs1NCwxNCBAQCBlbnZfZGljdCA9IHsqKm9zLmVudmly
b24sCiBpZiAiUEdfVEVTVF9FWFRSQSIgbm90IGluIGVudl9kaWN0IGFuZCBhcmdzLnBnX3Rl
c3RfZXh0cmE6CiAgICAgZW52X2RpY3RbIlBHX1RFU1RfRVhUUkEiXSA9IGFyZ3MucGdfdGVz
dF9leHRyYQogCisjIEFkZCBleHRyYSByZWdyZXNzIGFyZ3VtZW50cyBiZWZvcmUgd2UgYWRk
IG5vbi1vcHRpb24gYXJndW1lbnRzCitpZiBhcmdzLnRlc3RuYW1lIGluIFsncmVncmVzcycs
ICdpc29sYXRpb24nLCAnZWNwZyddOgorICAgIGlmICdURU1QX0NPTkZJRycgaW4gZW52X2Rp
Y3Q6CisgICAgICAgIGFyZ3MudGVzdF9jb21tYW5kICs9IFsnLS10ZW1wLWNvbmZpZz0nICsg
ZW52X2RpY3RbJ1RFTVBfQ09ORklHJ11dCisKKyAgICBpZiAnRVhUUkFfUkVHUkVTU19PUFRT
JyBpbiBlbnZfZGljdDoKKyAgICAgICAgYXJncy50ZXN0X2NvbW1hbmQgKz0gc2hsZXguc3Bs
aXQoZW52X2RpY3RbJ0VYVFJBX1JFR1JFU1NfT1BUUyddKQorCiBpZiAiVEVTVFMiIGluIGVu
dl9kaWN0OgogICAgIGFyZ3MudGVzdF9jb21tYW5kICs9IGVudl9kaWN0WyJURVNUUyJdLnNw
bGl0KCkKIGVsc2U6Ci0tIAoyLjQ3LjMKCg==

--------------ZBKuT4gM8S92OEYypzmgLIf7--