Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Add 'make check-tests' behavior to the meson based builds

  1. Add support for EXTRA_REGRESS_OPTS for meson

    Andreas Karlsson <andreas@proxel.se> — 2025-02-27T12:53:17Z

    Hi,
    
    We use EXTRA_REGRESS_OPTS to make sure the whole test suite passes with 
    our extension loaded and since I prefer develop in meson over using 
    autotools and make the lack of support for EXTRA_REGRESS_OPTS in meson
    has bugged me for a while.
    
    I have implemented support for it as an environment variable we read in 
    the testwrap script instead of adding it as a configuration option to 
    meson.build. The reason for this is that I do not like having to run 
    "meson reconfigure" all the time plus that for the PG_TEST_EXTRA we 
    ended up having to add an environment variable anyway.
    
    To use this run e.g. the following:
    
       EXTRA_REGRESS_OPTS="--load-extension=pgcrypto" meson test
    
    Question: Would it make sense to rename it to PG_REGRESS_EXTRA_OPTS or 
    something similar while we already touch touch this code to make the 
    various options easier to remember?
    
    Andreas
    
  2. Re: Add support for EXTRA_REGRESS_OPTS for meson

    Andres Freund <andres@anarazel.de> — 2025-02-27T13:21:22Z

    Hi,
    
    On 2025-02-27 13:53:17 +0100, Andreas Karlsson wrote:
    > We use EXTRA_REGRESS_OPTS to make sure the whole test suite passes with our
    > extension loaded and since I prefer develop in meson over using autotools
    > and make the lack of support for EXTRA_REGRESS_OPTS in meson
    > has bugged me for a while.
    
    Yep, we should add support for that.  TEMP_CONFIG probably too.
    
    
    > Question: Would it make sense to rename it to PG_REGRESS_EXTRA_OPTS or
    > something similar while we already touch touch this code to make the various
    > options easier to remember?
    
    I'd not tackle that at the same time personally.
    
    
    > @@ -51,7 +52,12 @@ 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
    >
    > -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)
    >  # Meson categorizes a passing TODO test point as bad
    >  # (https://github.com/mesonbuild/meson/issues/13183).  Remove the TODO
    >  # directive, so Meson computes the file result like Perl does.  This could
    
    I hacked up something similar before, for TEMP_CONFIG, and found that I needed
    to do something like this:
    
    +if 'TEMP_CONFIG' in os.environ and \
    +    args.testname in ['regress', 'isolation', 'ecpg']:
    +        # be careful to insert before non-option args, otherwise it'll fail
    +        # e.g. on windows
    +        args.test_command.insert(1, '--temp-config='+os.environ['TEMP_CONFIG'])
    
    Greetings,
    
    Andres Freund
    
    
    
    
  3. Re: Add support for EXTRA_REGRESS_OPTS for meson

    vignesh C <vignesh21@gmail.com> — 2025-03-13T14:41:49Z

    On Thu, 27 Feb 2025 at 18:51, Andres Freund <andres@anarazel.de> wrote:
    >
    > Hi,
    >
    > On 2025-02-27 13:53:17 +0100, Andreas Karlsson wrote:
    > > We use EXTRA_REGRESS_OPTS to make sure the whole test suite passes with our
    > > extension loaded and since I prefer develop in meson over using autotools
    > > and make the lack of support for EXTRA_REGRESS_OPTS in meson
    > > has bugged me for a while.
    >
    > Yep, we should add support for that.  TEMP_CONFIG probably too.
    >
    >
    > > Question: Would it make sense to rename it to PG_REGRESS_EXTRA_OPTS or
    > > something similar while we already touch touch this code to make the various
    > > options easier to remember?
    >
    > I'd not tackle that at the same time personally.
    >
    >
    > > @@ -51,7 +52,12 @@ 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
    > >
    > > -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)
    > >  # Meson categorizes a passing TODO test point as bad
    > >  # (https://github.com/mesonbuild/meson/issues/13183).  Remove the TODO
    > >  # directive, so Meson computes the file result like Perl does.  This could
    >
    > I hacked up something similar before, for TEMP_CONFIG, and found that I needed
    > to do something like this:
    >
    > +if 'TEMP_CONFIG' in os.environ and \
    > +    args.testname in ['regress', 'isolation', 'ecpg']:
    > +        # be careful to insert before non-option args, otherwise it'll fail
    > +        # e.g. on windows
    > +        args.test_command.insert(1, '--temp-config='+os.environ['TEMP_CONFIG'])
    
    Could you please upload a v2 to address this? In the meantime, I’ve
    updated the commitfest entry status to 'Waiting on Author.' Kindly
    update the status once the new version is posted.
    
    Regards,
    Vignesh
    
    
    
    
  4. Re: Add support for EXTRA_REGRESS_OPTS for meson

    Rustam ALLAKOV <rustamallakov@gmail.com> — 2025-03-19T22:25:46Z

    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.
  5. Re: Add support for EXTRA_REGRESS_OPTS for meson

    Andreas Karlsson <andreas@proxel.se> — 2025-12-31T01:01:05Z

    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