diff mbox series

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

Message ID 20231102184154.46185-2-adam.duskett@amarulasolutions.com
State Rejected, archived
Headers show
Series [v2,1/2] package/postgresql/postgresql.service: set locale for initdb to C | expand

Commit Message

Adam Duskett Nov. 2, 2023, 6:41 p.m. UTC
Perform a basic check that performs the following:
  - Check if /var/lib/pgsql/postmaster.pid exists.
  - Check if 'psql -c SHOW server_version;' returns sucessfully.

Note: systemd takes quite a while to start up, so check the output of
      `systemctl is-active postgresql` until it shows "active" with a timeout
      of 15 seconds.

Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
---
v1 -> v2:
  - Drop ": str" in front of config (Thomas)

  - Drop BR2_PER_PACKAGE_DIRECTORIES (Thomas)

  - Rename TestPostgreSQLInitd to TestPostgreSQLInitSysV (Thomas)

  - Use BASIC_TOOLCHAIN_CONFIG for TestPostgreSQLInitSysV (Thomas)

  - Move self.assertRunOk("ls /var/lib/pgsql/postmaster.pid") to after
    `systemctl is-active postgresql` returns successfully. (Yann)

  - Add an explination as to why the systemd test is not using
    BASIC_TOOLCHAIN_CONFIG.

 DEVELOPERS                                    |  1 +
 .../testing/tests/package/test_postgresql.py  | 73 +++++++++++++++++++
 2 files changed, 74 insertions(+)
 create mode 100644 support/testing/tests/package/test_postgresql.py

Comments

Yann E. MORIN Dec. 18, 2023, 5:31 p.m. UTC | #1
Adam, All,

On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> Perform a basic check that performs the following:
>   - Check if /var/lib/pgsql/postmaster.pid exists.
>   - Check if 'psql -c SHOW server_version;' returns sucessfully.

Thanks for this new runtime test!

> Note: systemd takes quite a while to start up, so check the output of
>       `systemctl is-active postgresql` until it shows "active" with a timeout
>       of 15 seconds.

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

So, I don't think usingt is-active is a good way to test whether a
service is actually running.

Furthermore, the unit may be active, but the service might not yet be
ready to serve, so again I don;t think it is still valid to reply on it.

> Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> ---
>   - 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.

[--SNIP--]
> diff --git a/support/testing/tests/package/test_postgresql.py b/support/testing/tests/package/test_postgresql.py
> new file mode 100644
> index 0000000000..fa692bab7b
> --- /dev/null
> +++ b/support/testing/tests/package/test_postgresql.py
> @@ -0,0 +1,73 @@
> +import os
> +import time
> +import infra.basetest
> +
> +
> +class TestPostgreSQLInitSysV(infra.basetest.BRTest):
> +    config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
> +        """
> +        BR2_PACKAGE_POSTGRESQL=y
> +        BR2_TARGET_ROOTFS_EXT2=y
> +        BR2_TARGET_ROOTFS_EXT2_4=y
> +        BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> +        # BR2_TARGET_ROOTFS_TAR is not set
> +        """
> +
> +    def test_run(self):
> +        img = os.path.join(self.builddir, "images", "rootfs.ext2")
> +        self.emulator.boot(
> +            arch="armv5",
> +            kernel="builtin",
> +            options=["-drive", f"file={img},if=scsi,format=raw"],
> +            kernel_cmdline=["root=/dev/sda"])
> +        self.emulator.login()
> +
> +        # Check if the Daemon is running
> +        self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> +        # Check if we can connect to the database.
> +        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?

Regards,
Yann E. MORIN.

> +
> +class TestPostgreSQLInitSystemd(infra.basetest.BRTest):
> +    # Taken from test_systemd.py
> +    config = """
> +        BR2_arm=y
> +        BR2_cortex_a9=y
> +        BR2_ARM_ENABLE_VFP=y
> +        BR2_TOOLCHAIN_EXTERNAL=y
> +        BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
> +        BR2_INIT_SYSTEMD=y
> +        BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
> +        BR2_PACKAGE_POSTGRESQL=y
> +        BR2_TARGET_ROOTFS_EXT2=y
> +        BR2_TARGET_ROOTFS_EXT2_4=y
> +        BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> +        # BR2_TARGET_ROOTFS_TAR is not set
> +        """
> +
> +    def test_run(self):
> +        img = os.path.join(self.builddir, "images", "rootfs.ext2")
> +        self.emulator.boot(
> +            arch="armv7",
> +            kernel="builtin",
> +            options=["-drive", f"file={img},if=sd,format=raw"],
> +            kernel_cmdline=["root=/dev/mmcblk0"])
> +        self.emulator.login()
> +
> +        # It may take some time for PostgreSQL to finish startup. Give it at least 15 seconds.
> +        is_active = False
> +        for i in range(15):
> +            output, _ = self.emulator.run("systemctl is-active postgresql")
> +            if output[0] == "active":
> +                is_active = True
> +                break
> +            time.sleep(1)
> +        if not is_active:
> +            self.fail("postgresql failed to activate!")
> +
> +        # Check if the Daemon is running
> +        self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> +
> +        # Check if we can connect to the database.
> +        self.assertRunOk("cd / && su postgres -c \"psql -c 'SHOW server_version;'\"")
> -- 
> 2.41.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
Adam Duskett Dec. 18, 2023, 5:41 p.m. UTC | #2
Yann;

Thank you for reviewing the test.

On Mon, Dec 18, 2023 at 10:31 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Adam, All,
>
> On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> > Perform a basic check that performs the following:
> >   - Check if /var/lib/pgsql/postmaster.pid exists.
> >   - Check if 'psql -c SHOW server_version;' returns sucessfully.
>
> Thanks for this new runtime test!
>
> > Note: systemd takes quite a while to start up, so check the output of
> >       `systemctl is-active postgresql` until it shows "active" with a timeout
> >       of 15 seconds.
>
> 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...

>
> So, I don't think usingt is-active is a good way to test whether a
> service is actually running.
>
> Furthermore, the unit may be active, but the service might not yet be
> ready to serve, so again I don;t think it is still valid to reply on it.
>
> > Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > ---
> >   - Drop BR2_PER_PACKAGE_DIRECTORIES (Thomas)
>
We need a consensus on this. This shouldn't happen. Either it is OK to
enable PPD in tests or it isn't.

> 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.
>
> [--SNIP--]
> > diff --git a/support/testing/tests/package/test_postgresql.py b/support/testing/tests/package/test_postgresql.py
> > new file mode 100644
> > index 0000000000..fa692bab7b
> > --- /dev/null
> > +++ b/support/testing/tests/package/test_postgresql.py
> > @@ -0,0 +1,73 @@
> > +import os
> > +import time
> > +import infra.basetest
> > +
> > +
> > +class TestPostgreSQLInitSysV(infra.basetest.BRTest):
> > +    config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
> > +        """
> > +        BR2_PACKAGE_POSTGRESQL=y
> > +        BR2_TARGET_ROOTFS_EXT2=y
> > +        BR2_TARGET_ROOTFS_EXT2_4=y
> > +        BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> > +        # BR2_TARGET_ROOTFS_TAR is not set
> > +        """
> > +
> > +    def test_run(self):
> > +        img = os.path.join(self.builddir, "images", "rootfs.ext2")
> > +        self.emulator.boot(
> > +            arch="armv5",
> > +            kernel="builtin",
> > +            options=["-drive", f"file={img},if=scsi,format=raw"],
> > +            kernel_cmdline=["root=/dev/sda"])
> > +        self.emulator.login()
> > +
> > +        # Check if the Daemon is running
> > +        self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> > +        # Check if we can connect to the database.
> > +        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.
>
> Regards,
> Yann E. MORIN.
>
> > +
> > +class TestPostgreSQLInitSystemd(infra.basetest.BRTest):
> > +    # Taken from test_systemd.py
> > +    config = """
> > +        BR2_arm=y
> > +        BR2_cortex_a9=y
> > +        BR2_ARM_ENABLE_VFP=y
> > +        BR2_TOOLCHAIN_EXTERNAL=y
> > +        BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
> > +        BR2_INIT_SYSTEMD=y
> > +        BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
> > +        BR2_PACKAGE_POSTGRESQL=y
> > +        BR2_TARGET_ROOTFS_EXT2=y
> > +        BR2_TARGET_ROOTFS_EXT2_4=y
> > +        BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> > +        # BR2_TARGET_ROOTFS_TAR is not set
> > +        """
> > +
> > +    def test_run(self):
> > +        img = os.path.join(self.builddir, "images", "rootfs.ext2")
> > +        self.emulator.boot(
> > +            arch="armv7",
> > +            kernel="builtin",
> > +            options=["-drive", f"file={img},if=sd,format=raw"],
> > +            kernel_cmdline=["root=/dev/mmcblk0"])
> > +        self.emulator.login()
> > +
> > +        # It may take some time for PostgreSQL to finish startup. Give it at least 15 seconds.
> > +        is_active = False
> > +        for i in range(15):
> > +            output, _ = self.emulator.run("systemctl is-active postgresql")
> > +            if output[0] == "active":
> > +                is_active = True
> > +                break
> > +            time.sleep(1)
> > +        if not is_active:
> > +            self.fail("postgresql failed to activate!")
> > +
> > +        # Check if the Daemon is running
> > +        self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> > +
> > +        # Check if we can connect to the database.
> > +        self.assertRunOk("cd / && su postgres -c \"psql -c 'SHOW server_version;'\"")
> > --
> > 2.41.0
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  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
Adam Duskett Dec. 18, 2023, 6:10 p.m. UTC | #3
On Mon, Dec 18, 2023 at 10:41 AM Adam Duskett <aduskett@gmail.com> wrote:
>
> Yann;
>
> Thank you for reviewing the test.
>
> On Mon, Dec 18, 2023 at 10:31 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> >
> > Adam, All,
> >
> > On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> > > Perform a basic check that performs the following:
> > >   - Check if /var/lib/pgsql/postmaster.pid exists.
> > >   - Check if 'psql -c SHOW server_version;' returns sucessfully.
> >
> > Thanks for this new runtime test!
> >
> > > Note: systemd takes quite a while to start up, so check the output of
> > >       `systemctl is-active postgresql` until it shows "active" with a timeout
> > >       of 15 seconds.
> >
> > 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...
To add to this, running a query on pgsql would fail if pgsql isn't running,
so this is perfectly safe.

>
> >
> > So, I don't think usingt is-active is a good way to test whether a
> > service is actually running.
> >
> > Furthermore, the unit may be active, but the service might not yet be
> > ready to serve, so again I don;t think it is still valid to reply on it.
> >
> > > Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > > ---
> > >   - Drop BR2_PER_PACKAGE_DIRECTORIES (Thomas)
> >
> We need a consensus on this. This shouldn't happen. Either it is OK to
> enable PPD in tests or it isn't.
>
> > 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.
> >
> > [--SNIP--]
> > > diff --git a/support/testing/tests/package/test_postgresql.py b/support/testing/tests/package/test_postgresql.py
> > > new file mode 100644
> > > index 0000000000..fa692bab7b
> > > --- /dev/null
> > > +++ b/support/testing/tests/package/test_postgresql.py
> > > @@ -0,0 +1,73 @@
> > > +import os
> > > +import time
> > > +import infra.basetest
> > > +
> > > +
> > > +class TestPostgreSQLInitSysV(infra.basetest.BRTest):
> > > +    config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
> > > +        """
> > > +        BR2_PACKAGE_POSTGRESQL=y
> > > +        BR2_TARGET_ROOTFS_EXT2=y
> > > +        BR2_TARGET_ROOTFS_EXT2_4=y
> > > +        BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> > > +        # BR2_TARGET_ROOTFS_TAR is not set
> > > +        """
> > > +
> > > +    def test_run(self):
> > > +        img = os.path.join(self.builddir, "images", "rootfs.ext2")
> > > +        self.emulator.boot(
> > > +            arch="armv5",
> > > +            kernel="builtin",
> > > +            options=["-drive", f"file={img},if=scsi,format=raw"],
> > > +            kernel_cmdline=["root=/dev/sda"])
> > > +        self.emulator.login()
> > > +
> > > +        # Check if the Daemon is running
> > > +        self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> > > +        # Check if we can connect to the database.
> > > +        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.
> >
> > Regards,
> > Yann E. MORIN.
> >
> > > +
> > > +class TestPostgreSQLInitSystemd(infra.basetest.BRTest):
> > > +    # Taken from test_systemd.py
> > > +    config = """
> > > +        BR2_arm=y
> > > +        BR2_cortex_a9=y
> > > +        BR2_ARM_ENABLE_VFP=y
> > > +        BR2_TOOLCHAIN_EXTERNAL=y
> > > +        BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
> > > +        BR2_INIT_SYSTEMD=y
> > > +        BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
> > > +        BR2_PACKAGE_POSTGRESQL=y
> > > +        BR2_TARGET_ROOTFS_EXT2=y
> > > +        BR2_TARGET_ROOTFS_EXT2_4=y
> > > +        BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> > > +        # BR2_TARGET_ROOTFS_TAR is not set
> > > +        """
> > > +
> > > +    def test_run(self):
> > > +        img = os.path.join(self.builddir, "images", "rootfs.ext2")
> > > +        self.emulator.boot(
> > > +            arch="armv7",
> > > +            kernel="builtin",
> > > +            options=["-drive", f"file={img},if=sd,format=raw"],
> > > +            kernel_cmdline=["root=/dev/mmcblk0"])
> > > +        self.emulator.login()
> > > +
> > > +        # It may take some time for PostgreSQL to finish startup. Give it at least 15 seconds.
> > > +        is_active = False
> > > +        for i in range(15):
> > > +            output, _ = self.emulator.run("systemctl is-active postgresql")
> > > +            if output[0] == "active":
> > > +                is_active = True
> > > +                break
> > > +            time.sleep(1)
> > > +        if not is_active:
> > > +            self.fail("postgresql failed to activate!")
> > > +
> > > +        # Check if the Daemon is running
> > > +        self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> > > +
> > > +        # Check if we can connect to the database.
> > > +        self.assertRunOk("cd / && su postgres -c \"psql -c 'SHOW server_version;'\"")
> > > --
> > > 2.41.0
> > >
> > > _______________________________________________
> > > buildroot mailing list
> > > buildroot@buildroot.org
> > > https://lists.buildroot.org/mailman/listinfo/buildroot
> >
> > --
> > .-----------------.--------------------.------------------.--------------------.
> > |  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
Yann E. MORIN Dec. 18, 2023, 8:44 p.m. UTC | #4
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.
Adam Duskett Dec. 18, 2023, 9:40 p.m. UTC | #5
Yann;


On Mon, Dec 18, 2023 at 1:44 PM Yann E. MORIN <yann.morin.1998@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@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.  |
> '------------------------------^-------^------------------^--------------------'
diff mbox series

Patch

diff --git a/DEVELOPERS b/DEVELOPERS
index 26b25edd98..548cf25646 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -38,6 +38,7 @@  F:	package/flutter-gallery/
 F:	package/flutter-pi/
 F:	package/flutter-sdk-bin/
 F:	support/testing/tests/package/test_flutter.py
+F:	support/testing/tests/package/test_postgresql.py
 
 N:	Adam Heinrich <adam@adamh.cz>
 F:	package/jack1/
diff --git a/support/testing/tests/package/test_postgresql.py b/support/testing/tests/package/test_postgresql.py
new file mode 100644
index 0000000000..fa692bab7b
--- /dev/null
+++ b/support/testing/tests/package/test_postgresql.py
@@ -0,0 +1,73 @@ 
+import os
+import time
+import infra.basetest
+
+
+class TestPostgreSQLInitSysV(infra.basetest.BRTest):
+    config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
+        """
+        BR2_PACKAGE_POSTGRESQL=y
+        BR2_TARGET_ROOTFS_EXT2=y
+        BR2_TARGET_ROOTFS_EXT2_4=y
+        BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
+
+    def test_run(self):
+        img = os.path.join(self.builddir, "images", "rootfs.ext2")
+        self.emulator.boot(
+            arch="armv5",
+            kernel="builtin",
+            options=["-drive", f"file={img},if=scsi,format=raw"],
+            kernel_cmdline=["root=/dev/sda"])
+        self.emulator.login()
+
+        # Check if the Daemon is running
+        self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
+
+        # Check if we can connect to the database.
+        self.assertRunOk("su postgres -c \"psql -c 'SHOW server_version;'\"")
+
+
+class TestPostgreSQLInitSystemd(infra.basetest.BRTest):
+    # Taken from test_systemd.py
+    config = """
+        BR2_arm=y
+        BR2_cortex_a9=y
+        BR2_ARM_ENABLE_VFP=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
+        BR2_INIT_SYSTEMD=y
+        BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
+        BR2_PACKAGE_POSTGRESQL=y
+        BR2_TARGET_ROOTFS_EXT2=y
+        BR2_TARGET_ROOTFS_EXT2_4=y
+        BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
+
+    def test_run(self):
+        img = os.path.join(self.builddir, "images", "rootfs.ext2")
+        self.emulator.boot(
+            arch="armv7",
+            kernel="builtin",
+            options=["-drive", f"file={img},if=sd,format=raw"],
+            kernel_cmdline=["root=/dev/mmcblk0"])
+        self.emulator.login()
+
+        # It may take some time for PostgreSQL to finish startup. Give it at least 15 seconds.
+        is_active = False
+        for i in range(15):
+            output, _ = self.emulator.run("systemctl is-active postgresql")
+            if output[0] == "active":
+                is_active = True
+                break
+            time.sleep(1)
+        if not is_active:
+            self.fail("postgresql failed to activate!")
+
+        # Check if the Daemon is running
+        self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
+
+        # Check if we can connect to the database.
+        self.assertRunOk("cd / && su postgres -c \"psql -c 'SHOW server_version;'\"")