[Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test

[Adam Duskett]

Yann;


On Mon, Dec 18, 2023 at 1:44 PM Yann E. MORIN <yann.morin.1998 at free.fr> wrote:
>
> Adam, All,
>
> On 2023-12-18 10:41 -0700, Adam Duskett spake thusly:
> > On Mon, Dec 18, 2023 at 10:31 AM Yann E. MORIN <yann.morin.1998 at free.fr> wrote:
> [--SNIP--]
> > > I think that we already concluded that testing if a unit was active or
> > > not was not representing whether the service was actually running. In
> > > the fultter test case for example, the unit was active, but the flutter
> > > app was just keeping crashing again and again, so the test wsa failing
> > > to detect failure...
> > test_flutter.py:
> > cmd = "systemctl is-active flutter-gallery"
> > I am following what is in test_flutter.py...
>
> Exactly, and as I explained on IRC (and already explained, and reported
> with [0]), it does not work: the unit is active, but the application the
> service runs is constantly crashing.
>
> So no, having a unit reported as active is not an indication of success.
> It is merely an indication that systemd did schedule and start that
> unit, not that the service is running appropriately.
>
> [0] https://lore.kernel.org/buildroot/7cf874f127f56c8132862163aa2c4d44f6f39efe.1696091295.git.yann.morin.1998@free.fr/
>
> > > >   - Drop BR2_PER_PACKAGE_DIRECTORIES (Thomas)
> > > Actually, I disagree with Thomas here: we added support for PPD in the
> > > infra, so that if PPD is enabled in a test, then TLPB is done to speed
> > > up the test.
> > We need a consensus on this. This shouldn't happen. Either it is OK to
> > enable PPD in tests or it isn't.
>
> I would say we usually do not want it, unless the runtime test "take too
> long to run" and thus we enable PPD. But it has to be justified (even
> briefly) in the comit log (e.g. "enabled PPD to divide the build time by
> N"; N=3 seems the minimum to justify PPD, I'd say)
>
> [--SNIP--]
> > > > +        self.assertRunOk("su postgres -c \"psql -c 'SHOW server_version;'\"")
> > > I don't understand why we need to test for the PID file before we
> > > connect to the daemon. If the daemon is not running, we won't be able to
> > > connect to it, so the test will fail, so the PID file test is redundant,
> > > no?
> > I am being thorough. I can remove the check for the PID file if you want.
>
> I see. However, what I wonder is: what does it bring, from the
> integration in Buildroot perspective? What I mean is:
>
>   - a runtime test is meant to validate that the integration in
>     Buildroot is delivering a package that is working, and working as
>     expected, at least in its basic features;
>
>   - a runtime test shall fail when the expectations are not met, with a
>     proper report of what is broken.
>
> So, in this case, querying the DB should be enough to check that the
> daemon is running: if it is not running, then the query will fail with
> the appropriate message, which is going to be different from the case
> where the DB does not exist, or where the access to the DB has been
> denied by lack of proper credentials, or...
>
> If a failure to query the DB can't indeed differentiate the daemon-is-
> not-running case from the others, then indeed we need another way. But
> then we need explanations about that, however brief they be.
>
I'm going to be real with you. I don't even use pgsql in my projects.
I was trying
to help because I noticed that it was broken, but this is far too much
bikeshedding
and nitpicking and I don't have the desire nor time to redo the test.
I'll decline the
patch and if someone else wants to make a test they are free to do so.

Adam

> Regards,
> Yann E. MORIN.
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'