Hi Jiri!
2009/10/31 Jiří Paleček <[email protected]>:
> On Sat, 31 Oct 2009 07:39:54 +0100, Garrett Cooper <[email protected]>
> wrote:
>
>> On Fri, Oct 30, 2009 at 5:19 AM, Subrata Modak
>> <[email protected]> wrote:
>>>
>>> On Fri, 2009-10-30 at 13:53 +0100, Jiří Paleček wrote:
>>>>
>>>> On Fri, 30 Oct 2009 12:37:48 +0100, Subrata Modak
>>>> <[email protected]> wrote:
>>>>
>>>> > On Thu, 2009-10-29 at 18:12 +0000, JiříPaleček wrote:
>>>> >> Hello,
>>>> >>
>>>> >> Subrata Modak <subr...@...> writes:
>>>> >>
>>>> >> >
>>>> >> > On Wed, 2009-10-21 at 02:19 +0200, Jiri Palecek wrote:
>>>> >> > > Hello,
>>>> >> > >
>>>> >> > > this is another patch fixing bashisms in LTP tests (the fixes are
>>>> >> more or
>>>> >> less the same as in the previous
>>>> >> > patches, except for a few exceptions). Note that the patch is not
>>>> >> complete,
>>>> >> in the sense that there may
>>>> >> > remain further bashisms in the source even after applying the patch
>>>> >> (like use
>>>> >> of arrays, which is visible
>>>> >> > even from this patch).
>>>> >> > >
>>>> >> > > Regards
>>>> >> > > Jiri Palecek
>>>> >> > >
>>>> >> > > Signed-off-by: Jiri Palecek <jpale...@...>
>>>> >> >
>>>> >> > Hmm,. Some of them failed to apply. Can you please resend only the
>>>> >> error
>>>> >> > part(s):
>>>> >> >
>>>> >>
>>>> >> according to my git repository. these are the missing parts:
>>>> >>
>>>> >> Signed-off-by: Jiri Palecek <[email protected]>
>>>> >
>>>> > Then something wrong with my CVS:
>>>> >
>>>> > patching file testcases/kernel/fs/fs-bench/modaltr.sh
>>>> > Hunk #1 FAILED at 43.
>>>> > 1 out of 1 hunk FAILED -- saving rejects to file
>>>> > testcases/kernel/fs/fs-bench/modaltr.sh.rej
>>>> > patching file testcases/kernel/fs/mongo/test.sh
>>>> > Hunk #1 FAILED at 26.
>>>> > Hunk #2 FAILED at 52.
>>>> > Hunk #3 FAILED at 70.
>>>> > 3 out of 3 hunks FAILED -- saving rejects to file
>>>> > testcases/kernel/fs/mongo/test.sh.rej
>>>> > patching file testcases/network/tcp_cmds/netstat/netstat01
>>>> >
>>>> > Can you please verify ?
>>>>
>>>> It works for me in CVS, so I guess it was just a linewrap, whitespace or
>>>> something issue. See attachment for the original (uncrippled) patch.
>>>
>>> Yes, it works fine now :-)
>>> patching file testcases/kernel/fs/fs-bench/modaltr.sh
>>> patching file testcases/kernel/fs/mongo/test.sh
>>> patching file testcases/network/tcp_cmds/netstat/netstat01
>>
>> Sorry for replying late again, but just for future reference, there's
>> a function called is_root in $(abs_top_srcdir)/testcases/lib/cmdlib.sh
>> [in the install tree -- it gets put in
>> $(abs_top_builddir)/testcases/bin] that can be leveraged s.t. you
>> don't have to do $(id -ru) -eq 0 everywhere. There are some other
>> handy constructs and functions too that can be used for expediting
>> setup, execution, and teardown. The only thing that might be a
>> detractor for some legacy scripts is that I explicitly did set -u to
>> avoid QA issues with unset variables, so all variables must be defined
>> before including the cmd library.
>>
>> Using that script though would be helpful as it cuts down on a lot of
>> duplicate code and aims to be completely POSIX compliant. Some other
>> pluses:
>>
>> 1. incr_tst_count - increments $TST_COUNT appropriately.
>> 2. TCID is automatically set if not already provided.
>> 3. exists is a function which ensures that commands exist on a given
>> system before executing a test.
>> 4. Common SHELL_DEBUG logic.
>>
>> Just trying to make life a little easier for test writers and
>> maintainers if possible :]...
>
> Thanks for noting it. I think, however, that it would be much more valuable
> to create exact duplicate of the C test API (or a subset of it) for shell
> scripts, as this would permit to remove much of the hacky stuff about
> maintaining TST_COUNT and return value from the shell scripts.
Ok. I'll definitely take that into consideration, but it's up to the
test writer to determine how many `subtestcases' are being executed in
a master testcase.
> Looking at the file you mentioned, I have just a few remarks:
>
> tst_cleanup()
> {
> # Disable the trap EXIT handler.
> trap '' EXIT
> # To ensure set -u passes...
> TCtmp=${TCtmp:=}
>
> Why? Shouldn't it catch invalid use (eg. without doing tst_setup first)?
Yeah, that's useless if TCtmp is exported in tst_setup and tst_setup
is executed to completion (or at least this variable is exported
there) first. I may need to adjust the trap(1) to get this to function
in a better way.
> # Nuke the testcase temporary directory if it exists.
> [ -d "$TCtmp" ] && rm -rf "$TCtmp" (1)
> }
>
> tst_setup() {
>
> TST_COUNT=1
> TST_TOTAL=${TST_TOTAL:=1} (2)
> export TCID TST_COUNT TST_TOTAL
>
> for varname in TST_TOTAL; do
> if eval "test -z \"\$${varname}\""; then
> end_testcase "You must set ${varname} before calling setup()."
> fi
> done
>
> What is the sense of doing "test -z "$TST_TOTAL"" in such an elaborate way,
> especially when you reset TST_TOTAL in (2) so test -z $TST_TOTAL can never
> be true here?
I was trying to avoid duplicate logic, but if it makes sense to do:
if test "x$foo" = x ; then
fi
if test "x$foo" = x ; then
fi
and do some basic arithmetic on the values to ensure they're in range,
then I'll gladly rewrite it :]. The point was to avoid the nasty `you
need to defined variable X.Y.Z' message from tst_res*(3), etc, because
this is quick and resolves coding issues with the underlying
testcases.
> ...
>
> TCtmp=${TCtmp:=$TEMPDIR/$TC$$}
> # User wants a temporary sandbox to play with.
> if [ -n "$TCtmp" -a "$TCtmp" != "$TEMPDIR/$$" ] ; then
> test -d "$TCtmp" || mkdir -p "$TCtmp"
>
> What is so special about "$TEMPDIR/$$" that you refuse to create a directory
> with this name, but still delete it in (1)?
Good point; this is not very well thought-out logic that I implemented
on impulse a while ago (I think that the original logic test was
`"$TCtmp" != "$TEMPDIR"').
> ...
> }
>
> incr_tst_count()
> {
> : $(( TST_COUNT + 1 ))
> }
>
> Was this really meant this way (ie. as a code with no effect), or should it
> be "+="?
That's a really stupid mistake that I made. Fixed now (thanks!).
Thanks for the review!
-Garrett
------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
