Mail archive
alpine-devel

Re: [alpine-devel] Halt CI on missing testsuite/check

From: Natanael Copa <ncopa_at_alpinelinux.org>
Date: Tue, 24 Oct 2017 15:58:41 +0200

On Tue, 24 Oct 2017 15:46:15 +0200
Carlo Landmeter <clandmeter_at_gmail.com> wrote:

> Hi friends,
>
> I was going over the PR queue yesterday and found out that many of our
> fellow
> developers/contributors found their way to github and help us to bump
> versions
> of packages which are out of date. We appricate the time spend but it would
> be
> great if those contributors could also take care of the recent addition of
> testsuite support (aka abuild check).
>
> The idea is to add a check function to our apkbuilds and run tests that are
> created by upsteam in their scripts. Examples of such tests suites are
> "make check" and "make test". If for any reason upstream did not include any
> testsuites an alternative could be to just add a simple cmd --help >
> /dev/null
> to atleast make sure the resulting binaries actually work.
>
> If by any chance the included testsuites fail it would be nice if
> contributors
> would try to fix them (send patches upstream) or else atleast indicate why
> they
> fail and add a message to the commit log and set options to include !check.
>
> To actually gain attention regarding testsuites my suggestion would be to
> require testsuites (abuild check) on our current CI. I wrote a small patch
> [1]
> to let abuild halt if check is not found (currently it only issues a
> warning).
>

I paste the patch here for convenience:

From 97d1ed476225fdd2f86ce5c803d9b98097b0cd3c Mon Sep 17 00:00:00 2001
From: Carlo Landmeter <clandmeter_at_gmail.com>
Date: Tue, 24 Oct 2017 14:48:52 +0200
Subject: [PATCH] abuild: add env option to require tests

This adds an env option REQ_CHECK to be passed to require testsuites to
be run. This does not clutter getopts so can be safely removed
afterwards. This allows our CI infra to enforce testsuites.
---
 abuild.in | 6 ++++++
 1 file changed, 6 insertions(+)
diff --git a/abuild.in b/abuild.in
index 41b465d..5da040f 100644
--- a/abuild.in
+++ b/abuild.in
_at_@ -226,6 +226,12 @@ default_sanitycheck() {
 	check_secfixes_comment || return 1
 
 	makedepends_has 'g++' && ! options_has toolchain && warning "g++ should not be in makedepends"
+        
+	if [ -n "$REQ_CHECK" ]; then
+		(unset check; . ./APKBUILD; type check >/dev/null 2>&1) || \
+			die "Testsuites (abuild check) are required or needs to be explicitly disabled!"
+	fi
+
 	return 0
 }
 
-- 
2.14.2
I think this is a good idea.
Are we ok with the variable name "REQ_CHECK"?
Should probably use "$APKBUILD" instead of ./APKBUILD
I'm ok with it otherwise.
-nc
---
Unsubscribe:  alpine-devel+unsubscribe_at_lists.alpinelinux.org
Help:         alpine-devel+help_at_lists.alpinelinux.org
---
Received on Tue Oct 24 2017 - 15:58:41 GMT