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

["Yann E. MORIN" <yann.morin.1998@free.fr>]

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@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.

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.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot