On Fri, Feb 21, 2025 at 10:18 AM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > On Wed, Feb 19, 2025 at 4:23 PM Hayato Kuroda (Fujitsu) > <kuroda.hay...@fujitsu.com> wrote: > > > > 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? > > > > Thanks Hayato for pointing it out. The test changes don't match my > expectations. As you rightly pointed out, I expected two (actually > three if needed) separate test files one for argument validation and > one for testing --database scenarios (the existing tests) and one more > for testing same scenarios when --all is specified. Right now all it > does is "# Verify that only user databases got subscriptions (not > template databases)". I expected testing the actual replication as > well like tests between lines around 527 and 582 in the latest > patchset. Those tests are performed when --database is subscribed. We > need similar tests performed when --all is specified. I didn't find > any of those tests being performed on node_s2. Given that the tests > for --databases and --all will be very similar, having them in the > same test file makes more sense. We also seem to be missing > $node_s2->teardown_node, do we? >
I agree with your point. Right now, my focus is on fixing the patch first, and I plan to split the test files at the end. That approach might help streamline the process. As of the latest patch attached at [1], this issue remains unresolved. I will proceed with splitting the tests once all other issues are addressed. [1] - https://www.postgresql.org/message-id/CAHv8RjJ9BsCg%2Bpur307b1JnfQebnmxFZLw4LdcGX7f-%3D6OK1vw%40mail.gmail.com Thanks and regards, Shubham Khanna.
