Date: Wed, 30 Apr 2014 10:28:03 -0700 From: Garrett Cooper <yaneurabeya@gmail.com> To: "Peel, Casey" <casey.peel@isilon.com> Cc: "freebsd-testing@freebsd.org" <freebsd-testing@freebsd.org>, "bdrewery@FreeBSD.org" <bdrewery@freebsd.org> Subject: Re: Please provide process for small, targeted fixes in tools/regression Message-ID: <CAGHfRMDoym5X3ME_snzgp0YOa3S-G5Zdko=QKeFcjNBEyaW7_A@mail.gmail.com> In-Reply-To: <16437CC5729B5345AF77F816513376E820BCBE25@MX103CL02.corp.emc.com> References: <16437CC5729B5345AF77F816513376E820BAF854@MX103CL02.corp.emc.com> <5F1D5D49-5F39-4EAC-89D5-E4D10FB3B01E@freebsd.org> <16437CC5729B5345AF77F816513376E820BAFE8E@MX103CL02.corp.emc.com> <16437CC5729B5345AF77F816513376E820BCBE25@MX103CL02.corp.emc.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Apr 30, 2014 at 10:03 AM, Peel, Casey <casey.peel@isilon.com> wrote: > Julio, here are 3 additional changes in tools/regression to get tests in there to run and pass via prove. > > * libutil.diff - fixes test-humanize_number.c to print 1-based test numbers instead of 0-based test numbers as prove expects. > * mmap.diff - adds .t file to allow running mmap test through prove > * fifo.diff - adds a top-level Makefile to compile the code in the fifo/ subdirectories. Adds .t to run the tests via prove. These changes look ok. The only thing (just a consistency nit) is that I would choose a consistent way of incrementing in loops, i.e. i=$(expr $i + 1) : $(( i += 1 )) i=$(( $i + 1 )) e.g. $ cat increment.sh ; sh increment.sh #!/bin/sh set -x sh -c 'i=$(expr $i + 1); echo $i' sh -c ': $(( i += 1 )); echo $i' sh -c 'i=$(( $i + 1 )); echo $i' + sh -c 'i=$(expr $i + 1); echo $i' 1 + sh -c ': $(( i += 1 )); echo $i' 1 + sh -c 'i=$(( $i + 1 )); echo $i' 1 On the other hand, if it's all going to be converted over to ATF in the near future, it doesn't really matter :). If you (or Peter) commit the work to isilon-atf, then I'll do the work required to convert it (minus the libutil testcases -- these need to be merged into what's in isilon-atf from NetBSD). Thanks! -Garrett
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGHfRMDoym5X3ME_snzgp0YOa3S-G5Zdko=QKeFcjNBEyaW7_A>