From owner-freebsd-testing@FreeBSD.ORG Wed Jan 22 23:36:53 2014 Return-Path: Delivered-To: freebsd-testing@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 5C0E33FD for ; Wed, 22 Jan 2014 23:36:53 +0000 (UTC) Received: from mail-qc0-f177.google.com (mail-qc0-f177.google.com [209.85.216.177]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 12BB81034 for ; Wed, 22 Jan 2014 23:36:52 +0000 (UTC) Received: by mail-qc0-f177.google.com with SMTP id i8so1511582qcq.8 for ; Wed, 22 Jan 2014 15:36:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:content-type:mime-version:subject:from :in-reply-to:date:cc:content-transfer-encoding:message-id:references :to; bh=dCmsMr4Yb+QSVJYQM0mDk3TvvUR78Yp+V4zaD77D8Xo=; b=b5ayY5zYAHpXOEwKRauWuQpkURgYwRXsTxLUjwZ2LKPYrSjhA7Zn0XrEoU1q/vUD4N Uoa+bbGgauQAFxOuFrTpE2MOfue7UpH/WxwDWtKeSp6AoraJ87YIH3BJLzKfAlscEXBa Ui66dnhgSFuU8rai1xdPH204jHXPKYP/gQ4wx8BM5lUPLCT3u34eDxi0Lg1LOO005bSs 6LilmW6sH8DajgE+tsiU2RFCh21SEVOe4u4j1iLdIAJ9jQVG8fdJv3CDAD/4OYhwtuW+ cD1Yv2LN36jlkDFoP5dWuMa+qzBKTwtej8o9F4I0BfHjLO1Xz6Kjh0zzFdazxjkCra0U bywg== X-Gm-Message-State: ALoCoQlQi8WWH4yuWzA+QhAcLE5uhr8JZt8+vUXOi/KrFVagLgqMmnxiEaa7jE2IUJxkpDIo++4P X-Received: by 10.224.147.75 with SMTP id k11mr6946320qav.22.1390433811762; Wed, 22 Jan 2014 15:36:51 -0800 (PST) Received: from barcelona.nyc.corp.google.com (barcelona.nyc.corp.google.com [172.26.105.74]) by mx.google.com with ESMTPSA id j65sm5965467qgj.18.2014.01.22.15.36.50 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 22 Jan 2014 15:36:50 -0800 (PST) Content-Type: text/plain; charset=windows-1252 Mime-Version: 1.0 (Mac OS X Mail 7.1 \(1827\)) Subject: Re: [PATCH] convert /bin/sh tests over to ATF From: Julio Merino In-Reply-To: Date: Wed, 22 Jan 2014 18:36:48 -0500 Content-Transfer-Encoding: quoted-printable Message-Id: <2222060C-CA92-41E5-9B10-9EA075A2B96B@meroh.net> References: To: Garrett Cooper X-Mailer: Apple Mail (2.1827) Cc: freebsd-testing@FreeBSD.org, Jilles Tjoelker X-BeenThere: freebsd-testing@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Testing on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Jan 2014 23:36:53 -0000 On Jan 20, 2014, at 11:44, Garrett Cooper wrote: > The attached patch converts the /bin/sh tests using the existing = framework bin/sh/tests. > All of the tests pass except locale.0, and that=92s due to some = [currently] unknown issue with my environment =97 see misc/181151). I = could make some of the entries more permanent, but I was trying to make = the tests easier to backport by using shell globs. > Also, I added some functionality so the tests expect failures on = certain versions of FreeBSD due to missing functionality or conformance = modifications (see the .valid_osreldate files). > Thoughts? Comments? > Thanks! > -Garrett Pasted your diff inline for easier commenting but trimmed stuff. > diff --git a/bin/sh/tests/Makefile b/bin/sh/tests/Makefile > index f6ddb8a..e1424f5 100644 > --- a/bin/sh/tests/Makefile > +++ b/bin/sh/tests/Makefile > @@ -4,15 +4,8 @@ > =20 > TESTSDIR=3D ${TESTSBASE}/bin/sh > =20 > -TAP_TESTS_SH=3D legacy_test > -TAP_TESTS_SH_SED_legacy_test=3D -e 's,__SH__,/bin/sh,g' > -# Some tests in here are silently not run when the tests are executed = as > -# root. Explicitly tell Kyua to drop privileges. > -# > -# TODO(jmmv): Kyua needs to do this by default, not only when = explicitly > -# requested. See https://code.google.com/p/kyua/issues/detail?id=3D6 > -TEST_METADATA.legacy_test+=3D required_user=3D"unprivileged" > +KYUAFILE=3D auto KYUAFILE=3Dauto is the default; remove here and anywhere else. =20 > -SUBDIR+=3D builtins errors execution expansion parameters parser = set-e > +ATF_TESTS_SUBDIRS+=3D builtins errors execution expansion = parameters parser set-e > =20 > -.include > +.include > diff --git a/bin/sh/tests/Makefile.inc b/bin/sh/tests/Makefile.inc > new file mode 100644 > index 0000000..0a79fd8 > --- /dev/null > +++ b/bin/sh/tests/Makefile.inc > @@ -0,0 +1,51 @@ > +# $FreeBSD$ > + > +CLEANFILES=3D regress.sh > + > +KYUAFILE=3D auto > + > +SH_TESTS!=3D cd ${.CURDIR} && find -Es . -regex '.*\.[0-9]+' | sed -e = 's,^\./,,g' > + > +SH_TESTS_FILES!=3D cd ${.CURDIR} && find . \( -type f -and \! -name = Makefile\* \) | \ > + sed -e 's,^\./,,g' This won't work well if you don't use an OBJDIR, will it? > + > +TESTSDIR=3D ${TESTSBASE}/bin/sh/${.CURDIR:T} > + > +FILES+=3D ${SH_TESTS_FILES} > +FILESDIR=3D ${TESTSDIR} > + > +regress.sh: ../Makefile.inc ${SH_TESTS_FILES} > + @echo '' > ${.TARGET} Don't write directly to ${.TARGET} if you have more than one command = doing so. Otherwise, if the target fails half-way through (or is = interrupted), a second invocation of make won't properly regenerate the = file. You can either do: @{ \ echo ... \ echo ... \ ... \ } >${.TARGET} or create a .tmp file and move "atomically" to the target at the end. > + @echo 'OSRELDATE=3D$$(sysctl -n kern.osreldate)' >> ${.TARGET} > + @echo ': $${SH=3D/bin/sh}' >> ${.TARGET} > + @echo 'export SH' >> ${.TARGET} > + @echo '' >> ${.TARGET} > +.for test in ${SH_TESTS} > + @echo 'atf_test_case ${test:S/\//_/g:S/./__/g:S/-/____/g}' >> = ${.TARGET} > + @echo '${test:S/\//_/g:S/./__/g:S/-/____/g}_body() {' >> = ${.TARGET} > + @echo ' cd $$(atf_get_srcdir)' >> ${.TARGET} > +.if exists(${test}.valid_osreldate) > + @echo -n ' [ '`cat ${.CURDIR}/${test}.valid_osreldate`' ] = && ' \ > + >> ${.TARGET} > + @echo 'atf_expect_fail "expecting failure based on OS version"' = \ > + >> ${.TARGET} > +.endif > + @echo -n ' atf_check -s exit:${test:E} ' >> ${.TARGET} > +.if exists(${test}.stderr) > + @echo -n '-e "file:$$(atf_get_srcdir)/${test}.stderr" ' >> = ${.TARGET} > +.endif > +.if exists(${test}.stdout) > + @echo -n '-o "file:$$(atf_get_srcdir)/${test}.stdout" ' >> = ${.TARGET} > +.endif > + @echo '$${SH} "./${test}"' >> ${.TARGET} > + @echo '}' >> ${.TARGET} > + @echo '' >> ${.TARGET} > +.endfor > + @echo '' >> ${.TARGET} > + @echo 'atf_init_test_cases() {' >> ${.TARGET} > +.for test in ${SH_TESTS} > + @echo ' atf_add_test_case ${test:S/\//_/g:S/./__/g:S/-/____/g}' = >> ${.TARGET} > +.endfor > + @echo '}' >> ${.TARGET} All this logic in a Makefile is too complex and should really not be = there. Just create a simple shell script that defines test cases dynamically = based on the available files and install that instead. It's shell after = all, so you can define functions at run time with ease! The results = will be much easier to digest. ---- But now, on a more abstract level... This approach does not fit the "model" of atf-sh -- nor the model of = most unit testing libraries for that matter. An individual test program = for every test case is overkill and keeping all these tiny files around = is suboptimal. The following page has more details on this and the = corresponding rationales: https://wiki.freebsd.org/TestSuite/Structure I recommend to not do this in this manner. Instead, what I'd do is the = following: First, define one test program for every directory that currently = exists. This would leave you with builtins_test errors_test = execution_test expansion_test parameters_test parser_test set-e_test, = all in /usr/tests/bin/sh. There is no reason to put them in separate = subdirectories. Second, make each of these top-level test programs "auto-discover" the = various data files in their corresponding subdirectories, define an = individual test case for each, and run them "as is". For example, = builtins_test would inspect srcdir/builtins/*.0, dynamically define an = atf_test_case, and run the test code from within it. These two steps change the "glue code" to be atf and expose the = individual test cases to kyua with minimal changes to the existing code. And third, separately once the above works, consider merging the = contents of the individual files into the main test program. I'm not = sure keeping tons of small files is worth it, but that's a separate = discussion once you have got the basic atf structure in place. This is all very similar to what was done with the ipf tests in NetBSD. = You can take a look at them here: = http://cvsweb.netbsd.org/bsdweb.cgi/src/tests/ipf/?only_with_tag=3DMAIN (Before this was done, something similar to what you did above was = attempted by using awk to generate test programs... it was a nightmare = to maintain.) Hope this helps and thanks for looking into this!=