Date: Thu, 23 Jan 2014 19:38:26 -0800 From: Garrett Cooper <yaneurabeya@gmail.com> To: Julio Merino <julio@meroh.net> Cc: freebsd-testing@FreeBSD.org, Jilles Tjoelker <jilles@stack.nl> Subject: Re: [PATCH] convert /bin/sh tests over to ATF Message-ID: <2F20BCCB-1464-495B-B08A-B2C538B27D18@gmail.com> In-Reply-To: <2222060C-CA92-41E5-9B10-9EA075A2B96B@meroh.net> References: <B5290C1B-F262-479C-8D4F-A5D8B3CE5A52@gmail.com> <2222060C-CA92-41E5-9B10-9EA075A2B96B@meroh.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Jan 22, 2014, at 3:36 PM, Julio Merino <julio@meroh.net> wrote:
> On Jan 20, 2014, at 11:44, Garrett Cooper <yaneurabeya@gmail.com> =
wrote:
>=20
>> 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>
>=20
> Pasted your diff inline for easier commenting but trimmed stuff.
=85
> KYUAFILE=3Dauto is the default; remove here and anywhere else.
Fixed =97 thanks :)!
>> +SH_TESTS_FILES!=3D cd ${.CURDIR} && find . \( -type f -and \! -name =
Makefile\* \) | \
>> + sed -e 's,^\./,,g'
>=20
> This won't work well if you don't use an OBJDIR, will it?
Yeah, it probably wouldn=92t work too well with ${SH_TESTS} being =
defined the way it is. I overlooked the point when I last updated things =
by continuing to use that variable...
>> +
>> +TESTSDIR=3D ${TESTSBASE}/bin/sh/${.CURDIR:T}
>> +
>> +FILES+=3D ${SH_TESTS_FILES}
>> +FILESDIR=3D ${TESTSDIR}
>> +
>> +regress.sh: ../Makefile.inc ${SH_TESTS_FILES}
>> + @echo '' > ${.TARGET}
>=20
> 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.
>=20
> You can either do:
>=20
> @{ \
> echo ... \
> echo ... \
> ... \
> } >${.TARGET}
>=20
> or create a .tmp file and move "atomically" to the target at the end.
Good point. I=92d use the latter method (in this case) because it would =
be too hairy/convoluted if I tried to use inline shell logic.
...
> All this logic in a Makefile is too complex and should really not be =
there.
>=20
> 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.
>=20
> ----
>=20
> But now, on a more abstract level...
>=20
> 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:
>=20
> https://wiki.freebsd.org/TestSuite/Structure
>=20
> I recommend to not do this in this manner. Instead, what I'd do is =
the following:
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> This is all very similar to what was done with the ipf tests in =
NetBSD. You can take a look at them here:
>=20
> =
http://cvsweb.netbsd.org/bsdweb.cgi/src/tests/ipf/?only_with_tag=3DMAIN
>=20
> (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.)
>=20
> Hope this helps and thanks for looking into this!
All valuable input. I=92ll definitely think about it and apply your =
comments appropriately :).
Thanks!
-Garrett=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2F20BCCB-1464-495B-B08A-B2C538B27D18>