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

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Adam,

On Wed, 11 Oct 2023 11:22:50 +0200
Adam Duskett <adam.duskett@amarulasolutions.com> wrote:

> diff --git a/support/testing/tests/package/test_firewalld.py b/support/testing/tests/package/test_firewalld.py
> new file mode 100644
> index 0000000000..4b0282d3e4
> --- /dev/null
> +++ b/support/testing/tests/package/test_firewalld.py
> @@ -0,0 +1,94 @@
> +"""Test firewalld for both systemd and sysvinit."""
> +import os
> +import infra.basetest
> +
> +
> +class TestFirewalldSystemd(infra.basetest.BRTest):
> +    """Build the kernel as firewalld requires several the nftable options."""
> +
> +    __test__ = True

Are you sure this is needed?

> +    config: str = """

Please use:

	config = """

> +        BR2_arm=y
> +        BR2_cortex_a9=y
> +        BR2_ARM_ENABLE_VFP=y
> +        BR2_TOOLCHAIN_EXTERNAL=y
> +        BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
> +        BR2_PER_PACKAGE_DIRECTORIES=y

We typically don't user per-package directories for runtime tests. I
don't want to see a random set of runtime tests use PPD, and another
random set not use PPD.

> +    def test_run(self):
> +        cpio_file = os.path.join(self.builddir, "images", "rootfs.cpio")
> +        kernel_file = os.path.join(self.builddir, "images", "zImage")
> +        dtb_file = os.path.join(self.builddir, "images", "vexpress-v2p-ca9.dtb")
> +        self.emulator.boot(arch="armv7",
> +                           kernel=kernel_file,
> +                           kernel_cmdline=["console=ttyAMA0,115200"],
> +                           options=[
> +                               "-initrd", cpio_file,
> +                               "-dtb", dtb_file,
> +                               "-M", "vexpress-a9"
> +                           ])
> +        # It takes quite some time for the system to boot with firewalld,
> +        self.emulator.timeout_multiplier *= 10

I don't think we should modify the timeout_multiplier. The
timeout_multiplier is meant to be provided on the command line, to
adjust for the slowness of the local machine.

Instead I would prefer that self.emulator.login() gets a timeout
argument to override the current value of 60 seconds.

support/testing/tests/init/test_systemd_selinux.py and
support/testing/tests/package/test_lxc.py should be fixed as well, as
they weak the timeout_multiplier.

Also support/testing/tests/package/test_python_django.py is wrong,
because it does:

        timeout = 35 * self.emulator.timeout_multiplier
[...]
        self.assertRunOk(cmd, timeout=timeout)

This gets re-multiplied by self.emulator.timeout_multiplier in
self.emulator.run().

Could you rework this?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot