Dear Shubham,
Thanks for updating the patch!
> I have added a test case for non-dry-run mode to ensure that
> replication slots, subscriptions, and publications work as expected
> when '--all' is specified. Additionally, I have split the test file
> into two parts:
> 1) Command-line sanity checks – Validates the correctness of option parsing.
> 2) '--all' functionality and behavior – Verifies the health of
> subscriptions and ensures that --all specific scenarios, such as
> non-template databases not being subscribed to, are properly handled.
> This should improve test coverage while keeping the structure manageable.
TBH, I feel your change does not separate the test file into the two parts. ISTM
you just added some validation checks and a test how --all option works.
Ashutosh, does it match your suggestion?
Anyway, here are my comments.
01.
```
+ int num_rows;
```
I think num_rows is not needed, we can do like:
```
/* Process the query result */
- num_rows = PQntuples(res);
- for (int i = 0; i < num_rows; i++)
+ for (int i = 0; i < PQntuples(res); i+
```
And
```
/* Error if no databases were found on the source server */
- if (num_rows == 0)
+ if (num_dbs == 0)
```
02.
```
+ opt.all = false;
```
This must be done after initializing recovery_timeout.
03.
```
+# Set up node S1 as standby linking to node P
+$node_p->backup('backup_3');
+my $node_s1 = PostgreSQL::Test::Cluster->new('node_s1');
```
I do not like the name "S1" because this is the second standby sever
from the node_p.
04.
```
+# Verify that only user databases got subscriptions (not template databases)
+my @user_dbs = ($db1, $db2);
+foreach my $dbname (@user_dbs)
+{
+ my $sub_exists =
+ $node_s1->safe_psql($dbname, "SELECT count(*) FROM pg_subscription;");
+ is($sub_exists, '3', "Subscription created successfully for $dbname");
+}
```
Hmm, what do you want to check here? pg_subscription is a global catalog so that
very loop will see exactly the same content. Also, 'postgres' is also the user
database.
I feel you must ensure that all three databases (postgres, $db1, and $db2) have
a
subscription here.
05.
```
+# Verify replication is working
+$node_p->safe_psql($db1, "INSERT INTO tbl1 VALUES ('replication test');");
+
+$result = $node_s1->safe_psql($db1, "SELECT * FROM tbl1 ORDER BY 1");
+is( $result, qq(first row
+replication test
+second row), "replication successful in $db1");
+
+$node_s1->stop;
# Run pg_createsubscriber on node S. --verbose is used twice
# to show more information.
command_ok(
```
I'm not sure the part is not needed. This have already been checked in other
parts, right?
Best regards,
Hayato Kuroda
FUJITSU LIMITED
