Date: Wed, 22 Jan 2014 18:36:48 -0500 From: Julio Merino <julio@meroh.net> To: Garrett Cooper <yaneurabeya@gmail.com> Cc: freebsd-testing@FreeBSD.org, Jilles Tjoelker <jilles@stack.nl> Subject: Re: [PATCH] convert /bin/sh tests over to ATF Message-ID: <2222060C-CA92-41E5-9B10-9EA075A2B96B@meroh.net> In-Reply-To: <B5290C1B-F262-479C-8D4F-A5D8B3CE5A52@gmail.com> References: <B5290C1B-F262-479C-8D4F-A5D8B3CE5A52@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Jan 20, 2014, at 11:44, Garrett Cooper <yaneurabeya@gmail.com> 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<convert-bin-sh-over-to-atf.patch>
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 <tap.test.mk>
> +.include <atf.test.mk>
> 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!=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2222060C-CA92-41E5-9B10-9EA075A2B96B>