~alpine/devel

2 2

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

Carlo Landmeter <clandmeter@gmail.com>
Details
Message ID
<CA+cSEmNptoEO1O3L+3T-uHFOULKYoYx6HDouJNrxM8M1vmJd+A@mail.gmail.com>
Sender timestamp
1508852775
DKIM signature
missing
Download raw message
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).


1. http://tpaste.us/40l1


-carlo
Natanael Copa <ncopa@alpinelinux.org>
Details
Message ID
<20171024155841.506079c2@ncopa-desktop.copa.dup.pw>
In-Reply-To
<CA+cSEmNptoEO1O3L+3T-uHFOULKYoYx6HDouJNrxM8M1vmJd+A@mail.gmail.com> (view parent)
Sender timestamp
1508853521
DKIM signature
missing
Download raw message
Patch: +6 -0
On Tue, 24 Oct 2017 15:46:15 +0200
Carlo Landmeter <clandmeter@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@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

	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@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---
Carlo Landmeter <clandmeter@gmail.com>
Details
Message ID
<CA+cSEmPMAwTF5Lzr++UjLWgsrGJOnm5QpkpgegjViUoFyWKN4A@mail.gmail.com>
In-Reply-To
<20171024155841.506079c2@ncopa-desktop.copa.dup.pw> (view parent)
Sender timestamp
1508881294
DKIM signature
missing
Download raw message
>
>
> I think this is a good idea.
>
> Are we ok with the variable name "REQ_CHECK"?
>

I can change it to REQUIRE_CHECK if thats more clear.


>
> Should probably use "$APKBUILD" instead of ./APKBUILD
>

ok


>
> I'm ok with it otherwise.
>
> -nc
>
Reply to thread Export thread (mbox)