From owner-freebsd-testing@FreeBSD.ORG Fri Jan 24 03:38:30 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 B747DD10 for ; Fri, 24 Jan 2014 03:38:30 +0000 (UTC) Received: from mail-pb0-x22f.google.com (mail-pb0-x22f.google.com [IPv6:2607:f8b0:400e:c01::22f]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 892DB1E00 for ; Fri, 24 Jan 2014 03:38:30 +0000 (UTC) Received: by mail-pb0-f47.google.com with SMTP id rp16so2690087pbb.34 for ; Thu, 23 Jan 2014 19:38:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=9Pkkm/y9F/oK0sXG5evGYoL+2l5+/ATo2aLGm1D2YnQ=; b=WCiB9DjSAawaM3BUC+ZTSbhWfHxpsi6mXQYeRVr3+xN2iA+OS3/keXSzlfS66EotmR h3tRaSg53+nWYF8iOmKF+2UNGKKlOzXdq5DWYk0hED8zAyt4d000hznWU2KdE1tZ763c XOZ64/gP8g1byxCeLiZg95upu/o3R/Ot6/2Yuv1yloKCB40mc2iLD0XVxJbXYdj1qljO zbuSo17FVKUSMLImtULA2W8NLv8GRFD9jYiSJZ5RUy+2BQJovzpEnUIPjdVapwgRGw/i JjLegfGAXq35MkoOqynPEtYcZZIBzL1okjIGABKI1hyfXMWfeA3hVQATZ9EArMcNzpV2 pNSg== X-Received: by 10.66.156.137 with SMTP id we9mr11971519pab.30.1390534710256; Thu, 23 Jan 2014 19:38:30 -0800 (PST) Received: from [192.168.20.5] (c-50-181-163-89.hsd1.wa.comcast.net. [50.181.163.89]) by mx.google.com with ESMTPSA id qq5sm44611444pbb.24.2014.01.23.19.38.28 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 23 Jan 2014 19:38:29 -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: Garrett Cooper In-Reply-To: <2222060C-CA92-41E5-9B10-9EA075A2B96B@meroh.net> Date: Thu, 23 Jan 2014 19:38:26 -0800 Content-Transfer-Encoding: quoted-printable Message-Id: <2F20BCCB-1464-495B-B08A-B2C538B27D18@gmail.com> References: <2222060C-CA92-41E5-9B10-9EA075A2B96B@meroh.net> To: Julio Merino 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: Fri, 24 Jan 2014 03:38:30 -0000 On Jan 22, 2014, at 3:36 PM, Julio Merino wrote: > On Jan 20, 2014, at 11:44, Garrett Cooper = 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 >=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=