Add --enable-bin-unit-tests

We lost C unit tests with the Rust-as-main massive buildsystem rework. But we're not going to RIIR tomorrow, and there's definitely a need to continue unit testing directly some of the C/C++ code. Add a new buildsystem option `--enable-bin-unit-tests` and turn it on for CI (but not production builds) *or* when compiling in `--debug` mode. This basically compiles the unit testing code into the main binary. It's hooked up to `make check` too.
morgaesis · Sep 7, 2021 · 6712aec · 6712aec
1 parent 5567810
commit 6712aec
Show file tree

Hide file tree

Showing 12 changed files with 93 additions and 23 deletions.
diff --git a/.cci.jenkinsfile b/.cci.jenkinsfile
@@ -29,13 +29,20 @@ parallel insttests: {
     def nhosts = 6
     def mem = (nhosts * 1024) + 512
     cosaPod(runAsUser: 0, memory: "${mem}Mi", cpu: "${nhosts}") {
-      stage("Build FCOS") {
+      stage("Unit tests") {
         checkout scm
         unstash 'rpms'
         // run this stage first without installing deps, so we match exactly the cosa pkgset
         // (+ our built rpm-ostree)
         shwrap("""
           dnf install -y *.rpm
+          # Cross check we enabled the unit tests
+          rpm-ostree --version | grep bin-unit-tests
+          rpm-ostree testutils c-units
+        """)
+      }
+      stage("Build FCOS") {
+        shwrap("""
           coreos-assembler init --force https://github.com/coreos/fedora-coreos-config
           # include our built rpm-ostree in the image
           mkdir -p overrides/rpm

diff --git a/Makefile-tests.am b/Makefile-tests.am
@@ -29,6 +29,7 @@ check-local:
 	@echo " \"make check\" only runs unit tests, which have limited coverage currently."
 	@echo " See HACKING.md for more information about VM-based integration testing."
 	@echo "  *** NOTE ***"
+	./rpm-ostree testutils c-units
 
 .PHONY: vmsync vmoverlay vmcheck testenv
 

diff --git a/ci/build-check.sh b/ci/build-check.sh
@@ -10,5 +10,5 @@ dn=$(dirname $0)
 # Hard fail on compiler warnings in CI.  We control our compiler
 # version as part of the coreos-assembler buildroot and expect
 # that to be clean.
-CONFIGOPTS="--enable-werror" ${dn}/build.sh
+CONFIGOPTS="--enable-werror --enable-bin-unit-tests" ${dn}/build.sh
 make check
diff --git a/ci/coreosci-rpmbuild.sh b/ci/coreosci-rpmbuild.sh
@@ -4,6 +4,7 @@ set -euo pipefail
 dn=$(dirname $0)
 . ${dn}/libbuild.sh
 
+set -x
 make -f .copr/Makefile srpm
-./packaging/rpmbuild-cwd --rebuild packaging/*.src.rpm
+./packaging/rpmbuild-cwd --with bin-unit-tests --rebuild packaging/*.src.rpm
 mv $(arch)/*.rpm .
diff --git a/configure.ac b/configure.ac
@@ -27,18 +27,52 @@ dnl if not set, which we definitely want; cmake doesn't do that.
 AC_PROG_CXX
 AM_PROG_CC_C_O
 
+RPM_OSTREE_FEATURES=""
+AC_SUBST([RPM_OSTREE_FEATURES])
+
+AC_MSG_CHECKING(whether to build in debug mode)
+debug_release=release
+if $(echo $CFLAGS |grep -q -E "(-O0|-Og)"); then
+  debug_release=debug
+fi
+AC_MSG_RESULT($debug_release)
+
+dnl These bits based on gnome:librsvg/configure.ac
+dnl By default, we build in release mode (but without LTO!)
+AC_ARG_ENABLE(rust-debug,
+  AC_HELP_STRING([--enable-rust-debug],
+    [Build Rust code with debugging information [default=no]]),
+    [rust_debug_release=$enableval],
+    [rust_debug_release=$debug_release])
+dnl Canonicalize yes/no to debug/release
+AS_IF([test x$rust_debug_release = xno ], [rust_debug_release=release])
+AS_IF([test x$rust_debug_release = xyes ], [rust_debug_release=debug])
+
 AC_ARG_ENABLE(sanitizers,
               AS_HELP_STRING([--enable-sanitizers],
                              [Enable ASAN and UBSAN (default: no)]),,
               [enable_sanitizers=no])
 AM_CONDITIONAL(BUILDOPT_ASAN, [test x$enable_sanitizers != xno])
 
+AC_ARG_ENABLE(bin-unit-tests,
+              AS_HELP_STRING([--enable-bin-unit-tests],
+                             [(default: yes if debug build, no for release build)]),,
+              [enable_bin_unit_tests=maybe])
+case "${enable_bin_unit_tests}-${debug_release}" in
+  maybe-debug) enable_bin_unit_tests=yes;;
+  maybe-*) enable_bin_unit_tests=no;;
+esac
+
+AS_IF([test x$enable_bin_unit_tests = xyes], [
+  AC_DEFINE([BUILDOPT_BIN_UNIT_TESTS], 1, [Define if unit tests are injected into the binary])
+  RPM_OSTREE_FEATURES="$RPM_OSTREE_FEATURES bin-unit-tests"
+])
+AM_CONDITIONAL(BUILDOPT_BIN_UNIT_TESTS, [test x$enable_bin_unit_tests = xyes])
+
 # Initialize libtool
 LT_PREREQ([2.2.4])
 LT_INIT([disable-static])
 
-RPM_OSTREE_FEATURES=""
-AC_SUBST([RPM_OSTREE_FEATURES])
 
 PKG_PROG_PKG_CONFIG
 
@@ -102,23 +136,6 @@ AS_IF([test -z "$rustc"], [AC_MSG_ERROR([rustc is required for --enable-rust])])
 dnl See comment in installdeps.sh
 AM_CONDITIONAL(BUILDOPT_PREBUILT_BINDINGS, [test -f rpmostree-cxxrs-prebuilt.h])
 
-AC_MSG_CHECKING(whether to build in debug mode)
-debug_release=release
-if $(echo $CFLAGS |grep -q -E "(-O0|-Og)"); then
-  debug_release=debug
-fi
-AC_MSG_RESULT($debug_release)
-
-dnl These bits based on gnome:librsvg/configure.ac
-dnl By default, we build in release mode (but without LTO!)
-AC_ARG_ENABLE(rust-debug,
-  AC_HELP_STRING([--enable-rust-debug],
-    [Build Rust code with debugging information [default=no]]),
-    [rust_debug_release=$enableval],
-    [rust_debug_release=$debug_release])
-dnl Canonicalize yes/no to debug/release
-AS_IF([test x$rust_debug_release = xno ], [rust_debug_release=release])
-AS_IF([test x$rust_debug_release = xyes ], [rust_debug_release=debug])
 
 RUST_TARGET_SUBDIR=${rust_debug_release}
 AC_SUBST([RUST_TARGET_SUBDIR])
@@ -137,6 +154,7 @@ AC_OUTPUT
 echo "
     $PACKAGE $VERSION
 
+    features:                                $RPM_OSTREE_FEATURES
     introspection:                           $found_introspection
     ASAN + UBSAN:                            ${enable_sanitizers:-no}
     gtk-doc:                                 $enable_gtk_doc

diff --git a/packaging/rpm-ostree.spec.in b/packaging/rpm-ostree.spec.in
@@ -24,6 +24,8 @@ BuildRequires: rust
 
 # Enable ASAN + UBSAN
 %bcond_with sanitizers
+# Embedded unit tests
+%bcond_with bin_unit_tests
 
 # RHEL8 doesn't ship zchunk today.  See also the comments
 # in configure.ac around this as libdnf/librepo need to be in
@@ -156,7 +158,7 @@ env NOCONFIGURE=1 ./autogen.sh
 %if 0%{?build_rustflags:1}
 export RUSTFLAGS="%{build_rustflags}"
 %endif
-%configure --disable-silent-rules --enable-gtk-doc %{?rpmdb_default} %{?with_sanitizers:--enable-sanitizers}
+%configure --disable-silent-rules --enable-gtk-doc %{?rpmdb_default} %{?with_sanitizers:--enable-sanitizers}  %{?with_bin_unit_tests:--enable-bin-unit-tests}
 
 %make_build
 

diff --git a/rust/src/lib.rs b/rust/src/lib.rs
@@ -540,6 +540,7 @@ pub mod ffi {
         fn early_main();
         fn rpmostree_main(args: &[&str]) -> Result<i32>;
         fn rpmostree_process_global_teardown();
+        fn c_unit_tests() -> Result<()>;
     }
 
     unsafe extern "C++" {

diff --git a/rust/src/testutils.rs b/rust/src/testutils.rs
@@ -50,6 +50,8 @@ enum Opt {
     GenerateSyntheticUpgrade(SyntheticUpgradeOpts),
     /// Validate that we can parse the output of `rpm-ostree status --json`.
     ValidateParseStatus,
+    /// Run the C unit tests
+    CUnits,
     /// Test that we can 🐄
     Moo,
 }
@@ -266,6 +268,7 @@ pub(crate) fn testutils_entrypoint(args: Vec<String>) -> CxxResult<()> {
     match opt {
         Opt::GenerateSyntheticUpgrade(ref opts) => update_os_tree(opts)?,
         Opt::ValidateParseStatus => validate_parse_status()?,
+        Opt::CUnits => crate::ffi::c_unit_tests()?,
         Opt::Moo => test_moo()?,
     };
     Ok(())

diff --git a/src/app/libmain.cxx b/src/app/libmain.cxx
@@ -540,4 +540,13 @@ rpmostree_process_global_teardown ()
   rpmostree_polkit_agent_close ();
 }
 
+// Execute all the C/C++ unit tests.
+void
+c_unit_tests ()
+{
+  // Add unit tests to a new C/C++ file here.
+  rpmostreed_utils_tests ();
+}
+
+
 } /* namespace */
diff --git a/src/app/rpmostreemain.h b/src/app/rpmostreemain.h
@@ -8,4 +8,6 @@ void early_main ();
 void rpmostree_process_global_teardown ();
 int rpmostree_main (rust::Slice<const rust::Str> args);
 
+void c_unit_tests ();
+
 }
diff --git a/src/daemon/rpmostreed-utils.cxx b/src/daemon/rpmostreed-utils.cxx
@@ -667,3 +667,26 @@ check_sd_inhibitor_locks (GCancellable    *cancellable,
     }
   return glnx_throw (error, "%s", error_msg->str);
 }
+
+#ifdef BUILDOPT_BIN_UNIT_TESTS
+static void
+test_refspec_parse_partial (void)
+{
+  g_autoptr(GError) local_error = NULL;
+  GError **error = &local_error;
+
+  g_autofree char *new_refspec = NULL;
+  rpmostreed_refspec_parse_partial ("baz:", "foo:bar", &new_refspec, error);
+  g_assert_no_error (local_error);
+  g_assert_cmpstr (new_refspec, ==, "baz:bar");
+  g_print ("ok %s\n", G_STRFUNC);
+}
+#endif
+
+void 
+rpmostreed_utils_tests (void)
+{
+#ifdef BUILDOPT_BIN_UNIT_TESTS
+  test_refspec_parse_partial ();
+#endif
+}
diff --git a/src/libpriv/rpmostree-util.h b/src/libpriv/rpmostree-util.h
@@ -368,4 +368,7 @@ rpmostree_variant_native_to_be (GVariant **v);
 char**
 rpmostree_cxx_string_vec_to_strv (rust::Vec<rust::String> &v);
 
+void 
+rpmostreed_utils_tests (void);
+
 G_END_DECLS