[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#955259: libjcat: Please add consider adding autopkgtest coverage



Source: libjcat
Version: 0.1.0-1
Severity: wishlist
Tags: patch
Control: block -1 by 955258

I noticed that libjcat has GNOME-style installed-tests, which in principle
make it easy to add autopkgtest coverage, so I tried to contribute a
trivial patch to do so. Unfortunately, the installed-tests don't actually
work when run installed (they rely on the original source tree), so it
turned out to be less trivial than I'd hoped.

I've attached patches, but the patch added in debian/patches should probably
be discussed with upstream. As well as making the test pass, it also fixes a
source of non-reproducibility.

For it to pass, the test for libjcat-dev requires the patches I attached
to #955258.

    smcv
>From c33068507c34a1416ee023f4064f19015f9b9c75 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Sat, 28 Mar 2020 17:37:36 +0000
Subject: [PATCH 7/9] Install test data so that the installed-tests can pass

---
 ...est-data-next-to-the-test-executable.patch | 273 ++++++++++++++++++
 debian/patches/series                         |   1 +
 2 files changed, 274 insertions(+)
 create mode 100644 debian/patches/Install-test-data-next-to-the-test-executable.patch

diff --git a/debian/patches/Install-test-data-next-to-the-test-executable.patch b/debian/patches/Install-test-data-next-to-the-test-executable.patch
new file mode 100644
index 0000000..6a8d473
--- /dev/null
+++ b/debian/patches/Install-test-data-next-to-the-test-executable.patch
@@ -0,0 +1,273 @@
+From: Simon McVittie <smcv@debian.org>
+Date: Sat, 28 Mar 2020 17:35:46 +0000
+Subject: Install test data next to the test executable
+
+This is conventional for GNOME-style installed-tests, and lets us find
+it with g_test_build_filename() (which uses the G_TEST_SRCDIR and
+G_TEST_BUILDDIR environment variables, or looks near the executable).
+
+The environment variables avoiding the need to hard-code the
+build directory into the test executable, which isn't suitable for
+reproducible builds.
+
+Signed-off-by: Simon McVittie <smcv@debian.org>
+---
+ data/tests/colorhug/meson.build |  5 +++--
+ data/tests/meson.build          |  5 +++++
+ data/tests/pki/meson.build      |  6 ++++++
+ libjcat/jcat-self-test.c        | 40 ++++++++++++++++++++--------------------
+ libjcat/meson.build             | 13 ++++++++-----
+ 5 files changed, 42 insertions(+), 27 deletions(-)
+
+diff --git a/data/tests/colorhug/meson.build b/data/tests/colorhug/meson.build
+index 1851fbf..475bf46 100644
+--- a/data/tests/colorhug/meson.build
++++ b/data/tests/colorhug/meson.build
+@@ -1,8 +1,9 @@
+ install_data([
+     'firmware.bin',
+     'firmware.bin.asc',
++    'firmware.bin.p7b',
+   ],
+-  install_dir: installed_test_datadir,
++  install_dir: installed_test_bindir / 'colorhug',
+ )
+ 
+ if get_option('pkcs7')
+@@ -17,6 +18,6 @@ if get_option('pkcs7')
+                         '--infile', '@INPUT@',
+                         '--outfile', '@OUTPUT@'],
+     install: true,
+-    install_dir: installed_test_datadir,
++    install_dir: installed_test_bindir / 'colorhug',
+   )
+ endif
+diff --git a/data/tests/meson.build b/data/tests/meson.build
+index 5bfca03..617edf2 100644
+--- a/data/tests/meson.build
++++ b/data/tests/meson.build
+@@ -15,5 +15,10 @@ pkcs7_privkey = custom_target('test-privkey.pem',
+ subdir('pki')
+ subdir('colorhug')
+ 
++install_data(
++  'meson.build',
++  install_dir: installed_test_bindir,
++)
++
+ testdatadir_src = meson.current_source_dir()
+ testdatadir_dst = meson.current_build_dir()
+diff --git a/data/tests/pki/meson.build b/data/tests/pki/meson.build
+index f35f2a9..f5bf31d 100644
+--- a/data/tests/pki/meson.build
++++ b/data/tests/pki/meson.build
+@@ -7,4 +7,10 @@ pkcs7_certificate = custom_target('test.pem',
+                       '--template', pkcs7_config,
+                       '--load-privkey', '@INPUT@',
+                       '--outfile', '@OUTPUT@'],
++  install_dir: installed_test_bindir / 'pki',
++)
++install_data(
++  'GPG-KEY-Linux-Vendor-Firmware-Service',
++  'LVFS-CA.pem',
++  install_dir: installed_test_bindir / 'pki',
+ )
+diff --git a/libjcat/jcat-self-test.c b/libjcat/jcat-self-test.c
+index 7c3558b..f7a8375 100644
+--- a/libjcat/jcat-self-test.c
++++ b/libjcat/jcat-self-test.c
+@@ -216,7 +216,7 @@ jcat_sha1_engine_func (void)
+ 	g_assert_cmpint (jcat_engine_get_verify_kind (engine), ==, JCAT_ENGINE_VERIFY_KIND_CHECKSUM);
+ 
+ 	/* verify checksum */
+-	fn_pass = g_build_filename (TESTDATADIR_SRC, "colorhug", "firmware.bin", NULL);
++	fn_pass = g_test_build_filename (G_TEST_DIST, "colorhug", "firmware.bin", NULL);
+ 	data_fwbin = jcat_get_contents_bytes (fn_pass, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fwbin);
+@@ -230,7 +230,7 @@ jcat_sha1_engine_func (void)
+ 	g_assert_cmpstr (jcat_result_get_authority (result_pass), ==, NULL);
+ 
+ 	/* verify will fail */
+-	fn_fail = g_build_filename (TESTDATADIR_SRC, "meson.build",NULL);
++	fn_fail = g_test_build_filename (G_TEST_DIST, "meson.build",NULL);
+ 	data_fail = jcat_get_contents_bytes (fn_fail, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fail);
+@@ -274,7 +274,7 @@ jcat_sha256_engine_func (void)
+ 	g_assert_cmpint (jcat_engine_get_verify_kind (engine), ==, JCAT_ENGINE_VERIFY_KIND_CHECKSUM);
+ 
+ 	/* verify checksum */
+-	fn_pass = g_build_filename (TESTDATADIR_SRC, "colorhug", "firmware.bin", NULL);
++	fn_pass = g_test_build_filename (G_TEST_DIST, "colorhug", "firmware.bin", NULL);
+ 	data_fwbin = jcat_get_contents_bytes (fn_pass, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fwbin);
+@@ -288,7 +288,7 @@ jcat_sha256_engine_func (void)
+ 	g_assert_cmpstr (jcat_result_get_authority (result_pass), ==, NULL);
+ 
+ 	/* verify will fail */
+-	fn_fail = g_build_filename (TESTDATADIR_SRC, "meson.build",NULL);
++	fn_fail = g_test_build_filename (G_TEST_DIST, "meson.build",NULL);
+ 	data_fail = jcat_get_contents_bytes (fn_fail, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fail);
+@@ -340,7 +340,7 @@ jcat_gpg_engine_func (void)
+ 
+ 	/* set up context */
+ 	jcat_context_set_keyring_path (context, "/tmp/libjcat-self-test/var");
+-	pki_dir = g_build_filename (TESTDATADIR_SRC, "pki", NULL);
++	pki_dir = g_test_build_filename (G_TEST_DIST, "pki", NULL);
+ 	jcat_context_add_public_keys (context, pki_dir);
+ 
+ 	/* get engine */
+@@ -356,7 +356,7 @@ jcat_gpg_engine_func (void)
+ 	g_assert_cmpstr (str, ==, str_perfect);
+ 
+ 	/* verify with GnuPG */
+-	fn_pass = g_build_filename (TESTDATADIR_SRC, "colorhug", "firmware.bin", NULL);
++	fn_pass = g_test_build_filename (G_TEST_DIST, "colorhug", "firmware.bin", NULL);
+ 	data_fwbin = jcat_get_contents_bytes (fn_pass, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fwbin);
+@@ -371,7 +371,7 @@ jcat_gpg_engine_func (void)
+ 			 "3FC6B804410ED0840D8F2F9748A6D80E4538BAC2");
+ 
+ 	/* verify will fail with GnuPG */
+-	fn_fail = g_build_filename (TESTDATADIR_SRC, "meson.build",NULL);
++	fn_fail = g_test_build_filename (G_TEST_DIST, "meson.build",NULL);
+ 	data_fail = jcat_get_contents_bytes (fn_fail, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fail);
+@@ -406,7 +406,7 @@ jcat_pkcs7_engine_func (void)
+ 
+ 	/* set up context */
+ 	jcat_context_set_keyring_path (context, "/tmp/libjcat-self-test/var");
+-	pki_dir = g_build_filename (TESTDATADIR_SRC, "pki", NULL);
++	pki_dir = g_test_build_filename (G_TEST_DIST, "pki", NULL);
+ 	jcat_context_add_public_keys (context, pki_dir);
+ 
+ 	/* get engine */
+@@ -417,11 +417,11 @@ jcat_pkcs7_engine_func (void)
+ 	g_assert_cmpint (jcat_engine_get_verify_kind (engine), ==, JCAT_ENGINE_VERIFY_KIND_SIGNATURE);
+ 
+ 	/* verify with a signature from the old LVFS */
+-	fn_pass = g_build_filename (TESTDATADIR_SRC, "colorhug", "firmware.bin", NULL);
++	fn_pass = g_test_build_filename (G_TEST_DIST, "colorhug", "firmware.bin", NULL);
+ 	data_fwbin = jcat_get_contents_bytes (fn_pass, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fwbin);
+-	fn_sig = g_build_filename (TESTDATADIR_SRC, "colorhug", "firmware.bin.p7b", NULL);
++	fn_sig = g_test_build_filename (G_TEST_DIST, "colorhug", "firmware.bin.p7b", NULL);
+ 	data_sig = jcat_get_contents_bytes (fn_sig, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_sig);
+@@ -434,7 +434,7 @@ jcat_pkcs7_engine_func (void)
+ 	g_assert_cmpstr (jcat_result_get_authority (result_pass), == , "O=Linux Vendor Firmware Project,CN=LVFS CA");
+ 
+ 	/* verify will fail with a self-signed signature */
+-	sig_fn2 = g_build_filename (TESTDATADIR_DST, "colorhug", "firmware.bin.p7c", NULL);
++	sig_fn2 = g_test_build_filename (G_TEST_BUILT, "colorhug", "firmware.bin.p7c", NULL);
+ 	blob_sig2 = jcat_get_contents_bytes (sig_fn2, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (blob_sig2);
+@@ -445,7 +445,7 @@ jcat_pkcs7_engine_func (void)
+ 	g_clear_error (&error);
+ 
+ 	/* verify will fail with valid signature and different data */
+-	fn_fail = g_build_filename (TESTDATADIR_SRC, "meson.build",NULL);
++	fn_fail = g_test_build_filename (G_TEST_DIST, "meson.build",NULL);
+ 	data_fail = jcat_get_contents_bytes (fn_fail, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fail);
+@@ -533,7 +533,7 @@ jcat_context_verify_blob_func (void)
+ 
+ 	/* set up context */
+ 	jcat_context_set_keyring_path (context, "/tmp");
+-	pki_dir = g_build_filename (TESTDATADIR_SRC, "pki", NULL);
++	pki_dir = g_test_build_filename (G_TEST_DIST, "pki", NULL);
+ 	jcat_context_add_public_keys (context, pki_dir);
+ 
+ 	/* get all engines */
+@@ -554,11 +554,11 @@ jcat_context_verify_blob_func (void)
+ 	g_clear_error (&error);
+ 
+ 	/* verify blob */
+-	fn_pass = g_build_filename (TESTDATADIR_SRC, "colorhug", "firmware.bin", NULL);
++	fn_pass = g_test_build_filename (G_TEST_DIST, "colorhug", "firmware.bin", NULL);
+ 	data_fwbin = jcat_get_contents_bytes (fn_pass, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fwbin);
+-	fn_sig = g_build_filename (TESTDATADIR_SRC, "colorhug", "firmware.bin.p7b", NULL);
++	fn_sig = g_test_build_filename (G_TEST_DIST, "colorhug", "firmware.bin.p7b", NULL);
+ 	data_sig = jcat_get_contents_bytes (fn_sig, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_sig);
+@@ -600,7 +600,7 @@ jcat_context_verify_item_sign_func (void)
+ 
+ 	/* set up context */
+ 	jcat_context_set_keyring_path (context, "/tmp");
+-	pki_dir = g_build_filename (TESTDATADIR_SRC, "pki", NULL);
++	pki_dir = g_test_build_filename (G_TEST_DIST, "pki", NULL);
+ 	jcat_context_add_public_keys (context, pki_dir);
+ 
+ 	/* get all engines */
+@@ -621,11 +621,11 @@ jcat_context_verify_item_sign_func (void)
+ 	g_clear_error (&error);
+ 
+ 	/* verify blob */
+-	fn_pass = g_build_filename (TESTDATADIR_SRC, "colorhug", "firmware.bin", NULL);
++	fn_pass = g_test_build_filename (G_TEST_DIST, "colorhug", "firmware.bin", NULL);
+ 	data_fwbin = jcat_get_contents_bytes (fn_pass, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fwbin);
+-	fn_sig = g_build_filename (TESTDATADIR_SRC, "colorhug", "firmware.bin.p7b", NULL);
++	fn_sig = g_test_build_filename (G_TEST_DIST, "colorhug", "firmware.bin.p7b", NULL);
+ 	data_sig = jcat_get_contents_bytes (fn_sig, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_sig);
+@@ -679,7 +679,7 @@ jcat_context_verify_item_csum_func (void)
+ 
+ 	/* set up context */
+ 	jcat_context_set_keyring_path (context, "/tmp");
+-	pki_dir = g_build_filename (TESTDATADIR_SRC, "pki", NULL);
++	pki_dir = g_test_build_filename (G_TEST_DIST, "pki", NULL);
+ 	jcat_context_add_public_keys (context, pki_dir);
+ 
+ 	/* get all engines */
+@@ -700,7 +700,7 @@ jcat_context_verify_item_csum_func (void)
+ 	g_clear_error (&error);
+ 
+ 	/* verify blob */
+-	fn_pass = g_build_filename (TESTDATADIR_SRC, "colorhug", "firmware.bin", NULL);
++	fn_pass = g_test_build_filename (G_TEST_DIST, "colorhug", "firmware.bin", NULL);
+ 	data_fwbin = jcat_get_contents_bytes (fn_pass, &error);
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fwbin);
+diff --git a/libjcat/meson.build b/libjcat/meson.build
+index 00b8cc7..5f8a3a8 100644
+--- a/libjcat/meson.build
++++ b/libjcat/meson.build
+@@ -221,14 +221,17 @@ if get_option('tests')
+     dependencies : [
+       libjcat_deps,
+     ],
+-    c_args : [
+-      '-DTESTDATADIR_SRC="' + testdatadir_src + '"',
+-      '-DTESTDATADIR_DST="' + testdatadir_dst + '"',
+-    ],
+     install : true,
+     install_dir : installed_test_bindir
+   )
+-  test('jcat-self-test', e)
++  test(
++    'jcat-self-test',
++    e,
++    env : [
++      'G_TEST_SRCDIR=' + testdatadir_src,
++      'G_TEST_BUILDDIR=' + testdatadir_dst,
++    ],
++  )
+ endif
+ 
+ jcat_incdir = include_directories('.')
diff --git a/debian/patches/series b/debian/patches/series
index d685412..edf69af 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,2 @@
 jcat-self-test-Sign-etc-os-release-instead-of-etc-machine.patch
+Install-test-data-next-to-the-test-executable.patch
-- 
2.26.0

>From 2f46af07dc67ed277a086af0fa28c37a3f54980b Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Sat, 28 Mar 2020 16:28:29 +0000
Subject: [PATCH 8/9] Add some simple autopkgtest coverage

Signed-off-by: Simon McVittie <smcv@debian.org>
---
 debian/tests/control       |  6 ++++++
 debian/tests/libjcat-dev   | 37 +++++++++++++++++++++++++++++++++++++
 debian/tests/libjcat-tests |  4 ++++
 3 files changed, 47 insertions(+)
 create mode 100644 debian/tests/control
 create mode 100755 debian/tests/libjcat-dev
 create mode 100755 debian/tests/libjcat-tests

diff --git a/debian/tests/control b/debian/tests/control
new file mode 100644
index 0000000..02684f1
--- /dev/null
+++ b/debian/tests/control
@@ -0,0 +1,6 @@
+Tests: libjcat-tests
+Depends: gnome-desktop-testing, libjcat-tests
+
+Tests: libjcat-dev
+Depends: build-essential, libjcat-dev, pkg-config
+Restrictions: allow-stderr superficial
diff --git a/debian/tests/libjcat-dev b/debian/tests/libjcat-dev
new file mode 100755
index 0000000..4f5fd22
--- /dev/null
+++ b/debian/tests/libjcat-dev
@@ -0,0 +1,37 @@
+#!/bin/sh
+# autopkgtest check: Build and run a program against libjcat, to verify
+# that the headers and pkg-config file are installed correctly
+# (C) 2012 Canonical Ltd.
+# (C) 2018-2020 Simon McVittie
+# Authors: Martin Pitt, Simon McVittie
+
+set -eux
+
+WORKDIR=$(mktemp -d)
+export XDG_RUNTIME_DIR="$WORKDIR"
+trap 'rm -rf "$WORKDIR"' 0 INT QUIT ABRT PIPE TERM
+cd "$WORKDIR"
+
+if [ -n "${DEB_HOST_GNU_TYPE:-}" ]; then
+    CROSS_COMPILE="${DEB_HOST_GNU_TYPE}-"
+else
+    CROSS_COMPILE=
+fi
+
+cat <<'EOF' > simple.c
+#include <jcat.h>
+
+int main(void)
+{
+    g_assert_cmpstr(jcat_blob_kind_to_string(JCAT_BLOB_KIND_SHA256), ==, "sha256");
+    return 0;
+}
+EOF
+
+# Deliberately word-splitting pkg-config's output:
+# shellcheck disable=SC2046
+"${CROSS_COMPILE}gcc" -o simple simple.c $("${CROSS_COMPILE}pkg-config" --cflags --libs jcat)
+echo "build: OK"
+[ -x ./simple ]
+./simple
+echo "run: OK"
diff --git a/debian/tests/libjcat-tests b/debian/tests/libjcat-tests
new file mode 100755
index 0000000..bdc6edf
--- /dev/null
+++ b/debian/tests/libjcat-tests
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+set -eu
+gnome-desktop-testing-runner libjcat
-- 
2.26.0

>From 4c8bf97f2b6bc3896b09bdf3c78a96ac2c846e52 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Sat, 28 Mar 2020 19:44:38 +0000
Subject: [PATCH 9/9] Update changelog

Signed-off-by: Simon McVittie <smcv@debian.org>
---
 debian/changelog | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/debian/changelog b/debian/changelog
index 525b60a..df85b11 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -6,8 +6,15 @@ libjcat (0.1.0-2) UNRELEASED; urgency=medium
       The autobuilders use a minimal chroot that doesn't necessarily have
       a machine ID, but base-files gives us /etc/os-release.
     (Closes: #955234)
+  * Fix -dev package dependencies:
+    - libjcat-dev: Add missing -dev dependencies for dependency libraries
+    - libjcat-dev: Add missing dependency on a matching libjcat1
+    - d/control: Enable gir debhelper sequence.
+      Otherwise ${gir:Depends} won't be generated.
+  * Install test data so that the installed-tests can pass
+  * Add some simple autopkgtest coverage
 
- -- Simon McVittie <smcv@debian.org>  Sat, 28 Mar 2020 16:48:23 +0000
+ -- Simon McVittie <smcv@debian.org>  Sat, 28 Mar 2020 19:43:32 +0000
 
 libjcat (0.1.0-1) unstable; urgency=medium
 
-- 
2.26.0


Reply to: