Horton

Thread

Commits

  1. pgbench: Use standard option handling test routines

  1. Missing program_XXX calling in pgbench tests

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-06-05T01:18:54Z

    Dear hackers,

While reviewing another thread I found #SUBJECT.

Most of client program executes program_help_ok, program_version_ok,
program_options_handling_ok() in their test, but pgbench does not do. Instead
pgbench has own tests for --help and --version options.

One concern is that program_help_ok() checks whether lines are too long or not.
For now, the rule seems to be kept but not sure in future.

I feel that we can replace tests with common function, like attached. How do you
think?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

  2. Re: Missing program_XXX calling in pgbench tests

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-06-05T01:50:36Z

    
On 2025/06/05 10:18, Hayato Kuroda (Fujitsu) wrote:
> Dear hackers,
> 
> While reviewing another thread I found #SUBJECT.
> 
> Most of client program executes program_help_ok, program_version_ok,
> program_options_handling_ok() in their test, but pgbench does not do. Instead
> pgbench has own tests for --help and --version options.
> 
> One concern is that program_help_ok() checks whether lines are too long or not.
> For now, the rule seems to be kept but not sure in future.
> 
> I feel that we can replace tests with common function, like attached. How do you
> think?

+1

A bit similar discussion came up before regarding pgbench and program_xxx_ok
in [1], but it seems that change was never applied.

Regards,

[1] https://postgr.es/m/20180723033518.GG2854@paquier.xyz

-- 
Fujii Masao
NTT DATA Japan Corporation

  3. RE: Missing program_XXX calling in pgbench tests

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-06-05T03:00:26Z

    Dear Fujii-san,

> A bit similar discussion came up before regarding pgbench and program_xxx_ok
> in [1], but it seems that change was never applied.

I didn't know that, thanks for sharing. ISTM, it tried to extend function to test the
shorter options.

While verifying the idea, I found that pg_config and pg_bsd_indent have not been
supported "-V" option yet. They must address the option ro improve test functions.

Attached patch set implemented the idea. 0001 is my original point and can be
backported. 0002-0004 needs API changes so they aim to be applied for HEAD.

If possible, I want to fork another thread to discuss 0002-0004 and want to
concentrate 0001 here.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

  4. Re: Missing program_XXX calling in pgbench tests

    Peter Eisentraut <peter@eisentraut.org> — 2025-06-05T07:19:48Z

    On 05.06.25 05:00, Hayato Kuroda (Fujitsu) wrote:
> Dear Fujii-san,
> 
>> A bit similar discussion came up before regarding pgbench and program_xxx_ok
>> in [1], but it seems that change was never applied.
> 
> I didn't know that, thanks for sharing. ISTM, it tried to extend function to test the
> shorter options.
> 
> While verifying the idea, I found that pg_config and pg_bsd_indent have not been
> supported "-V" option yet. They must address the option ro improve test functions.
> 
> Attached patch set implemented the idea. 0001 is my original point and can be
> backported. 0002-0004 needs API changes so they aim to be applied for HEAD.
> 
> If possible, I want to fork another thread to discuss 0002-0004 and want to
> concentrate 0001 here.

Patch 0001 looks very sensible.

I don't think we need to bother we the other ones.  pg_config works 
differently than the other programs anyway, because --version does not 
exit the program.  And pg_bsd_indent is an externally maintained 
program.  So I think it is ok if these two are intentionally different.

  5. RE: Missing program_XXX calling in pgbench tests

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-06-05T07:44:42Z

    Dear Peter,

Thanks for the comment.

> Patch 0001 looks very sensible.
>
> I don't think we need to bother we the other ones.  pg_config works
> differently than the other programs anyway, because --version does not
> exit the program.  And pg_bsd_indent is an externally maintained
> program.  So I think it is ok if these two are intentionally different.

You meant that 0002-0004 are not needed, right?
So let's put on out-of-scope...

Best regards,
Hayato Kuroda
FUJITSU LIMITED

  6. Re: Missing program_XXX calling in pgbench tests

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-06-06T11:22:43Z

    
On 2025/06/05 16:44, Hayato Kuroda (Fujitsu) wrote:
> Dear Peter,
> 
> Thanks for the comment.
> 
>> Patch 0001 looks very sensible.
>>
>> I don't think we need to bother we the other ones.  pg_config works
>> differently than the other programs anyway, because --version does not
>> exit the program.  And pg_bsd_indent is an externally maintained
>> program.  So I think it is ok if these two are intentionally different.
> 
> You meant that 0002-0004 are not needed, right?
> So let's put on out-of-scope...

I agree with Peter. I don't think patches 0002 and 0003 are necessary.

As for 0004, it adds tests for the short options -? and -V, which
duplicate the existing tests for the long options --help and --version.
I'm not sure it's worth adding tests just to confirm that the short
and long options behave the same.

Regards,

-- 
Fujii Masao
NTT DATA Japan Corporation

  7. RE: Missing program_XXX calling in pgbench tests

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-06-09T04:48:33Z

    Dear Fujii-san,

> I agree with Peter. I don't think patches 0002 and 0003 are necessary.
> 
> As for 0004, it adds tests for the short options -? and -V, which
> duplicate the existing tests for the long options --help and --version.
> I'm not sure it's worth adding tests just to confirm that the short
> and long options behave the same.

Only adding 0004 is not allowed because it can fail due to other commands.
So let's drop them.

I verified 0001 can be applied cleanly for all supported branches. To clarify,
let me attach the 0001 patch again. Please focus on it...

Best regards,
Hayato Kuroda
FUJITSU LIMITED

  8. Re: Missing program_XXX calling in pgbench tests

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-06-09T11:15:51Z

    
On 2025/06/09 13:48, Hayato Kuroda (Fujitsu) wrote:
> Dear Fujii-san,
> 
>> I agree with Peter. I don't think patches 0002 and 0003 are necessary.
>>
>> As for 0004, it adds tests for the short options -? and -V, which
>> duplicate the existing tests for the long options --help and --version.
>> I'm not sure it's worth adding tests just to confirm that the short
>> and long options behave the same.
> 
> Only adding 0004 is not allowed because it can fail due to other commands.
> So let's drop them.
> 
> I verified 0001 can be applied cleanly for all supported branches. To clarify,
> let me attach the 0001 patch again. Please focus on it...

+1 to focusing on the 0001 patch.

Since this isn't a bug fix, I'm not sure back-patching is strictly necessary.
That said, it does improve consistency and test coverage, e.g., by adding checks
like help text length, so I'd be fine with back-patching if others see value in it.

Regards,

-- 
Fujii Masao
NTT DATA Japan Corporation

  9. RE: Missing program_XXX calling in pgbench tests

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-06-12T03:23:40Z

    Dear Fujii-san,

> +1 to focusing on the 0001 patch.
> 
> Since this isn't a bug fix, I'm not sure back-patching is strictly necessary.
> That said, it does improve consistency and test coverage, e.g., by adding checks
> like help text length, so I'd be fine with back-patching if others see value in it.

Initially I thought this was helpful even for back branches, but it is not
100% needed.
No objections even if it is only applied to master - it can check new features in
future.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

  10. Re: Missing program_XXX calling in pgbench tests

    Peter Eisentraut <peter@eisentraut.org> — 2025-06-30T08:51:42Z

    On 12.06.25 05:23, Hayato Kuroda (Fujitsu) wrote:
>> +1 to focusing on the 0001 patch.
>>
>> Since this isn't a bug fix, I'm not sure back-patching is strictly necessary.
>> That said, it does improve consistency and test coverage, e.g., by adding checks
>> like help text length, so I'd be fine with back-patching if others see value in it.
> 
> Initially I thought this was helpful even for back branches, but it is not
> 100% needed.
> No objections even if it is only applied to master - it can check new features in
> future.

committed