On Mon, Aug 4, 2025 at 3:52 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
> On Sat, Aug 02, 2025 at 08:09:14AM +0900, Michael Paquier wrote:
>
> > Not the author of these two ones, but am I the only one who does not
> > see the point of these two tests? These check that something does not
> > happen,
> >
> > Wouldn't it be simpler to remove these checks
>
> I'm afraid that could lead to false positives or to the test taking longer as
> waiting for a timeout to occur.
This rationale is interesting. You are basically saying that you are
counting on one of the two log_contains() tests failing and never
actually reaching the pg_stat_database_conflicts query test in a
failure scenario.
When I was writing the recovery conflict (031_recovery_conflict.pl)
tests, I added both log message and stats checks because we didn't
have coverage of pg_stat_database_conflicts in our other stats tests
(that would have been hard to do in regress). The stats test was
expected to fail when something was wrong with the recovery conflict
stats updates -- not so much when something was wrong with the
recovery conflict handling logic itself. But that argument makes less
sense here in 035_standby_logical_decoding for the happy path test
case we are talking about. Though, I suppose someone could write code
that _incorrectly_ updated confl_active_logicalslot stats and this
would catch that.
Anyway, there's probably no reason to remove it (the
pg_stat_database_conflicts check for the happy path case) -- we could
come up with other, more impactful ways to get the test runtime down,
I imagine.
This test is not unprecedented in testing the absence of logfile
messages -- but it has the same problem those tests have in that they
don't fail nicely when the message changes or someone typo'd the
matching message string in the test.
- Melanie
