Re: Enhance 'pg_createsubscriber' to retrieve databases automatically when no database is provided.

On Wed, Feb 12, 2025 at 5:29 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Tue, Feb 11, 2025 at 9:16 PM Shubham Khanna
> <khannashubham1197(at)gmail(dot)com> wrote:
> >
> > > #13. Unanswered question "How are tests expecting this even passing?".
> > > Was a reason identified? IOW, how can we be sure the latest tests
> > > don't have a similar problem?
> > >
> >
> > In the v4-0001 patch [1], the tests were mistakenly using
> > 'command_fails' instead of 'command_fails_like' to verify failed test
> > cases. Since 'command_fails' only checks for a non-zero exit code and
> > not specific error messages, the tests were passing even when the
> > expected errors were not being triggered correctly.
> > To address this, I have updated the patch to use 'command_fails_like',
> > ensuring that the test cases now explicitly verify the correct failure
> > messages.
> >
>
> Ah, that makes sense. Thanks for sharing the reason. So in fact, it
> was a valid concern because the v5 was still carrying over the same
> flaw from v4... Anyway, it is good to know it is fixed now in v6.
>
> =====
>
> Some general comments for the patch v6-0001:
>
> Do you need to test every possible bad option combination? It may be
> fine because the error will be immediately raised so I expect the
> execution overhead to be ~zero.
>
> BTW, your bad option combination tests are only using --all-databases
> *after* the other options. Maybe you should mix it up a bit, sometimes
> putting it *before* the others as well, because rearranging will cause
> different errors.
>
> Everything else now looks good to me.
>

Fixed the given comments. The attached Patch contains the suggested changes.

Thanks and regards,
Shubham Khanna.