On Thu, Mar 20, 2025 at 4:53 PM Ashutosh Bapat
<ashutosh.bapat....@gmail.com> wrote:
>
> On Thu, Mar 20, 2025 at 10:25 AM Shubham Khanna
> <khannashubham1...@gmail.com> wrote:
>
>
> +# run pg_createsubscriber with '--database' and '--all' without '--dry-run'
> +# and verify the failure
> +command_fails_like(
> + [
> + 'pg_createsubscriber',
> + '--verbose',
> + '--pgdata' => $node_s->data_dir,
> + '--publisher-server' => $node_p->connstr($db1),
> + '--socketdir' => $node_s->host,
> + '--subscriber-port' => $node_s->port,
> + '--database' => $db1,
> + '--all',
> + ],
> + qr/--database cannot be used with --all/,
> + 'fail if --database is used with --all');
>
>
> Why does only this test not use --dry-run, but all other such tests
> use --dry-run? Either they should all use it or not use it.
>
I think this patch is adding a lot of extra tests which I don't think
are required. We should have one positive test in --dry-run mode
similar to ('run pg_createsubscriber without --databases') and
probably verify in the LOG that it has expanded commands for all
databases. Also, for negative tests, one or two tests are sufficient
instead of testing all possible combinations. I don't expect this new
option to add a noticeable testing time.
* <refsynopsisdiv>
+ <cmdsynopsis>
+ <command>pg_createsubscriber</command>
+ <arg rep="repeat"><replaceable>option</replaceable></arg>
+ <group choice="plain">
+ <group choice="req">
+ <arg choice="plain"><option>-a</option></arg>
+ <arg choice="plain"><option>--all</option></arg>
+ </group>
+ <group choice="req">
+ <arg choice="plain"><option>-D</option> </arg>
+ <arg choice="plain"><option>--pgdata</option></arg>
+ </group>
+ <replaceable>datadir</replaceable>
+ <group choice="req">
+ <arg choice="plain"><option>-P</option></arg>
+ <arg choice="plain"><option>--publisher-server</option></arg>
+ </group>
+ <replaceable>connstr</replaceable>
Most of this is unrelated to this patch. I suggest making a top-up
patch, we can commit it separately.
--
With Regards,
Amit Kapila.
