Date: Thu, 23 Jan 2014 14:16:32 -0800 From: Garrett Cooper <yaneurabeya@gmail.com> To: Julio Merino <julio@meroh.net> Cc: freebsd-testing@FreeBSD.org, Giorgos Keramidas <keramida@freebsd.org> Subject: Re: [PATCH] convert bin/date over to ATF Message-ID: <FA467521-BB53-47C0-BAE1-8BCAE29F922F@gmail.com> In-Reply-To: <B90AA730-B7B8-4F1A-9D07-67861C9A6401@meroh.net> References: <6079AD8F-5EBB-431C-A06B-9B51E2729F5A@gmail.com> <B90AA730-B7B8-4F1A-9D07-67861C9A6401@meroh.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Jan 22, 2014, at 3:56 PM, Julio Merino <julio@meroh.net> wrote:
> On Jan 20, 2014, at 13:40, Garrett Cooper <yaneurabeya@gmail.com> =
wrote:
>=20
>> This is based on work done by Giorgos a couple years ago.
>> Thanks!
>> -Garrett
>=20
> Pasting patch contents and commenting inline:
>=20
>> diff --git a/bin/date/tests/Makefile b/bin/date/tests/Makefile
>> index 540008b..459d019 100644
>> --- a/bin/date/tests/Makefile
>> +++ b/bin/date/tests/Makefile
>> @@ -4,6 +4,6 @@
>>=20
>> TESTSDIR=3D ${TESTSBASE}/bin/date
>>=20
>> -TAP_TESTS_SH=3D legacy_test
>> +ATF_TESTS_SH=3D regress
>=20
> Tests ought to end with _test per the description in =
https://wiki.freebsd.org/TestSuite/Structure
Ok! Is `_tests` ok?
> Also, "regress_test" is not a very indicative name. Will this only =
contain test cases for bugs to prevent regressions?
It should be =93date_functional_tests=94, because that=92s what they=92re =
doing.
> date_test or integration_test (due to the lack of unit tests for the =
code) may be better choices.
...
>> --- /dev/null
>> +++ b/bin/date/tests/regress.sh
>> @@ -0,0 +1,557 @@
>> +#!/bin/sh
>=20
> Remove. This comes from atf.test.mk automatically.
>=20
>> +
>> +#
>> +# Regression tests for date(1)
>> +#
>> +# Submitted by Edwin Groothuis <edwin@FreeBSD.org>
>> +#
>> +# $FreeBSD: src/tools/regression/bin/date/regress.sh,v 1.4 =
2011/01/09 22:05:09 keramida Exp $
>> +#
>> +
>> +#
>> +# These two date/times have been chosen carefully, they
>> +# create both the single digit and double/multidigit version of
>> +# the values.
>> +#
>> +# To create a new one, make sure you are using the UTC timezone!
>> +#
>> +
>> +TEST1=3D3222243 # 1970-02-07 07:04:03
>> +TEST2=3D1005600000 # 2001-11-12 21:11:12
>> +
>> +export LC_ALL=3DC
>> +export TZ=3DUTC
>=20
> Kyua does this already as part of the contract between atf and the =
runtime engine. You should not be resetting these.
Ok. This was code originally written with ATF in mind, but I totally =
agree.
>> +
>> +check()
>> +{
>> + S=3D$1
>> + A1=3D$2
>> + A2=3D$3
>=20
> S, A1, A2? What do these mean?
Not sure=85 giorgos wrote these (but I assume it=92s format_string, =
string, string_post_manipulation based on the comments=85).
> Also, make local.
Totally agree (will do)!
>> +
>> + # If the second sample text for formatted output has not been
>> + # passed, assume it should match exactly the first one.
>> + if [ -z "${A2}" ]; then
>> + A2=3D${A1}
>> + fi
>> +
>> + R=3D`date -r ${TEST1} +%${S}`
>=20
> Prefer $() over ``.
Me too (will change).
>> + atf_check test "${R}" =3D "${A1}"
>> +
>> + R=3D`date -r ${TEST2} +%${S}`
>> + atf_check test "${R}" =3D "${A2}"
>> +}
>> +
>> +# =
----------------------------------------------------------------------
>> +
>> +atf_test_case A
>> +A_head()
>> +{
>> + atf_set "descr" "Verifies that 'A' formatting spec works"
>=20
> These test case names are truly non-descriptive. I'd recommend =
renaming them to something like formatting_spec__A, etc. and omitting =
the definition of head() altogether. Will result on a much shorter test =
program, and being concise here for readability matters a lot.
>=20
> ... and then you can just create a "macro" to define test cases. Like =
this, but untested:
>=20
> formatting_spec_test_case() {
> local subname=3D"${1}"; shift
> local name=3D"formatting_spec__${subname}"
>=20
> atf_test_case "${name}"
> eval "${name}_body() { check ${*} }"
> }
>=20
> formatting_spec_test_case a a Sat Mon
> formatting_spec_test_case B B February November
> ...
> formatting_spec_test_case plus + "Sat Feb 7 07:04:03 UTC 1970" "Mon =
Nov 12 21:20:00 UTC 2001=94
Yeah. I=92ll think about that further and get back to you after I figure =
out how the code works a bit more (this was just a straight carryover =
from giorgos).
=85
Thanks!
-Garrett=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?FA467521-BB53-47C0-BAE1-8BCAE29F922F>