Date: Tue, 24 Oct 2017 15:58:41 +0200
From: Natanael Copa <ncopa@alpinelinux.org>
To: Carlo Landmeter <clandmeter@gmail.com>
Cc: Alpine-devel <alpine-devel@lists.alpinelinux.org>
Subject: Re: [alpine-devel] Halt CI on missing testsuite/check
Message-ID: <20171024155841.506079c2@ncopa-desktop.copa.dup.pw>
In-Reply-To: <CA+cSEmNptoEO1O3L+3T-uHFOULKYoYx6HDouJNrxM8M1vmJd+A@mail.gmail.com>
References: <CA+cSEmNptoEO1O3L+3T-uHFOULKYoYx6HDouJNrxM8M1vmJd+A@mail.gmail.com>
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: quoted-printable

On Tue, 24 Oct 2017 15:46:15 +0200
Carlo Landmeter <clandmeter@gmail.com> wrote:

> Hi friends,
>=20
> 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 wou=
ld
> be
> great if those contributors could also take care of the recent addition of
> testsuite support (aka abuild check).
>=20
> The idea is to add a check function to our apkbuilds and run tests that a=
re
> 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.
>=20
> 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 !chec=
k.
>=20
> 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).
>=20

I paste the patch here for convenience:

=46rom 97d1ed476225fdd2f86ce5c803d9b98097b0cd3c Mon Sep 17 00:00:00 2001
From: Carlo Landmeter <clandmeter@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
@@ -226,6 +226,12 @@ default_sanitycheck() {
 	check_secfixes_comment || return 1
=20
 	makedepends_has 'g++' && ! options_has toolchain && warning "g++ should n=
ot be in makedepends"
+       =20
+	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 d=
isabled!"
+	fi
+
 	return 0
 }
=20
--=20
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@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---