Re: [PATCH v2] unit: Skip tests that rely on PKCS#8 when parser is missing

On Thu, 2026-02-19 at 12:55 +0100, Marcel Holtmann wrote:
> Hi Bastien,
>=20
> > Add pre-check to tests that need the pkcs8_key_parser module loaded
> > (whether built-in or external), and allow tests that rely on it to
> > fail.
> >=20
> > This makes it possible to run the test suite successfully even on
> > systems where we might not be able to load modules if they're
> > missing
> > such as CIs, or other systems where we lack permissions.
> > ---
> >=20
> > Changes since v1:
> > - Use precheck test mechanism instead of flags
> >=20
> > unit/test-pem.c | 154 +++++++++++++++++++++++++++++++++------------
> > ---
> > 1 file changed, 105 insertions(+), 49 deletions(-)
> >=20
> > diff --git a/unit/test-pem.c b/unit/test-pem.c
> > index 7ee0597ed172..306e3a4f8656 100644
> > --- a/unit/test-pem.c
> > +++ b/unit/test-pem.c
> > @@ -10,6 +10,7 @@
> > #endif
> >=20
> > #include <assert.h>
> > +#include <sys/stat.h>
> >=20
> > #include <ell/ell.h>
> >=20
> > @@ -136,6 +137,18 @@ static const struct pem_from_data_test
> > single_line_cert_chain =3D {
> > "-----END CERTIFICATE-----\n",
> > };
> >=20
> > +static bool pkcs8_key_parser_precheck(const void *data)
> > +{
> > + struct stat s;
> > +
> > + /* Despite the path, this directory exists whether the module
> > + * is external or built-in. */
> > + if (stat ("/sys/module/pkcs8_key_parser", &s) !=3D 0)
> > + return false;
> > +
> > + return S_ISDIR (s.st_mode);
> > +}
> > +
>=20
> you don=E2=80=99t think doing it like this is better:
>=20
> +static bool test_load_precheck(const void *data)
> +{
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return true;
> +}
> +
> +#define add_file_test(name, data)
> l_test_add_data_func_precheck(name, data, \
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 test_load_file,
> test_load_precheck, 0);

It's a lot more work to undo something I already did. The types of
tests that are being run are also too heterogeneous to have one catch-
all macro, and we'll end up with a level of indirection where readers
try to figure out what macros do.

I don't find it more readable, but if you need this to land the patch,
I can make a v3.

> +
> =C2=A0int main(int argc, char *argv[])
> =C2=A0{
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 l_test_init(&argc, &argv);
> @@ -499,8 +507,7 @@ int main(int argc, char *argv[])
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 test_loa=
d_file,
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 TEST_LOA=
D_PARAMS("cert-entity-pkcs12-rc4-
> sha384.p12",
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 true, true, tr=
ue,
> true));
> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 l_test_add("pkcs#12/Combined PKCS#5=
 ciphers + SHA512",
> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 test_load_file=
,
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 add_file_test("pkcs#12/Combined PKC=
S#5 ciphers + SHA512",
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 TEST_LOA=
D_PARAMS("cert-entity-pkcs12-pkcs5-
> sha512.p12",
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 true, true, tr=
ue,
> true));
>=20
> I would most likely also push the TEST_LOAD_PARAMS into the
> add_file_test() macro, but seems a bit more work.
>=20
> Regards
>=20
> Marcel