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

Hi Bastien,

> 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);
> +}
> +

you don=E2=80=99t think doing it like this is better:

+static bool test_load_precheck(const void *data)
+{
+       return true;
+}
+
+#define add_file_test(name, data) l_test_add_data_func_precheck(name, =
data, \
+                                       test_load_file, =
test_load_precheck, 0);
+
 int main(int argc, char *argv[])
 {
        l_test_init(&argc, &argv);
@@ -499,8 +507,7 @@ int main(int argc, char *argv[])
                        test_load_file,
                        =
TEST_LOAD_PARAMS("cert-entity-pkcs12-rc4-sha384.p12",
                                                true, true, true, =
true));
-       l_test_add("pkcs#12/Combined PKCS#5 ciphers + SHA512",
-                       test_load_file,
+       add_file_test("pkcs#12/Combined PKCS#5 ciphers + SHA512",
                        =
TEST_LOAD_PARAMS("cert-entity-pkcs12-pkcs5-sha512.p12",
                                                true, true, true, =
true));

I would most likely also push the TEST_LOAD_PARAMS into the =
add_file_test() macro, but seems a bit more work.

Regards

Marcel