On Fri, Feb 21, 2025 at 10:18 AM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> On Wed, Feb 19, 2025 at 4:23 PM Hayato Kuroda (Fujitsu)
> <kuroda.hayato@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.
