Re: meson and check-tests

2025-03-22 Thread Rustam ALLAKOV
Hi, 

please note, same file `/src/tools/testwrap` on the same line number 
is being changed in this patch [1] in the same commitfest.

[1] https://commitfest.postgresql.org/patch/5602/

Regards,
Rustam Allakov

Re: Patch: Cover POSITION(bytea,bytea) with tests

2025-03-21 Thread Rustam ALLAKOV
The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, passed

Hi Aleksander, 
your patch looks good to me. 
Changing status to `Ready for Committer`

Regards,
Rustam Allakov

The new status of this patch is: Ready for Committer


Re: Add support for EXTRA_REGRESS_OPTS for meson

2025-03-27 Thread Rustam ALLAKOV
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)

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.

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

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

Regards.

Re: [PATCH] Refactor SLRU to always use long file names

2025-03-28 Thread Rustam ALLAKOV
The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, passed

Hi Aleksander, 
I have reviewed your patch. It looks good to me.

Kindest regards.
Rustam Allakov

The new status of this patch is: Ready for Committer