[Toybox] [PATCH SET] Fix test harness issues and fix tests

[Andy Chu]

OK here's the patch I was talking about... I actually wrote it in Make
first and then completely rewrote it, and it's much simpler now.

My rant is that Makefiles should be for dataflow and shell scripts are
for actions, and they work together beautifully (makefiles call shell
scripts, and sometimes shell scripts call 'make').

But I think .PHONY targets are usually a sign of abusing Makefiles as
shell scripts.  There's nothing really different about 'make test' vs
'./test.sh all'.

I think 'make test' should be kept as an alias for downstream
familiarity, but 'make test_sed' etc. should be removed in favor of
'./test.sh single sed'.  They are preserved in this patch though.

I would patch this and try ./test.sh single -asan expr.  It's super
nice and gives a stack trace of a memory leak.  It definitely found
leaks before your last patch with 'refree'.  I didn't try yet but I
think it will find leaks in the string -> int conversion (or it will
if we add the proper test cases.)

-msan is finding unitialized reads, like in xabspath(), *buf is used
without initializing char buf[4096].
-ubsan is finding some integer overflows, and some more stuff I didn't
look into deeply yet.

Note that this is runtime instrumentation, not static analysis, so
they are actually happening.

Also I think these renaming would be nice:

scripts/test.sh -> scripts/run_tests.sh  # helper script for "front
end" ./test.sh
scripts/runtest.sh -> scripts/test_lib.sh  # the thing all the tests source
scripts/install.c -> scripts/instlist.c ?  Because the binary is
generated/instlist?

(I didn't do that because I'm not sure how it would patch...)

Also there is a TODO in test.sh... there seems to be a couple places
where we are grepping toys/*/*.c for info about commands, and I think
one is wrong because I get '-toysh' in addition to 'toysh'.  This can
probably be cleaned up... if you want me to do that in this patch set
let me know.

Andy



On Sun, Mar 20, 2016 at 9:57 PM, Andy Chu <andychup at gmail.com> wrote:
> On Sun, Mar 20, 2016 at 9:08 PM, Rob Landley <rob at landley.net> wrote:
>> On 03/18/2016 08:24 PM, Andy Chu wrote:
>>> This is a pretty big cleanup, still in progress, but I think these
>>> parts are ready to merge.
>>
>> It's a large lump, some bits of which I disagree with. Should I apply
>> and then apply an undo patch, or chip off bits?
>
> Yeah sorry about the big lump, but the original history is super messy
> -- there was a lot of churn of going back and forth as I was figuring
> things out and learning the code.
>
> If you read the commit description, it seems disjointed, but
> everything is related to producing a "green build" (passing tests).
> If you compare "make test" before and after, all the changes show up
> there, and I think it's good progress.
>
> But I have another large lump coming up (call it patch 0004)!  It adds
> some significant functionality (ASAN and all that, which found some
> good bugs), as well as fixing test environment issues.
>
> There was a recurring theme in the test failures -- under "make test",
> you would have a directory in your PATH with ALL toybox binaries.
> Under "make test_sed", you would only have sed in that dir.  Patches
> 1-3 don't fix that but patch 4 does.
>
> Patch 0004 also addresses some of the issues below.  I changed it to
> "make generated/single/sed" instead of "make sed", and this eliminates
> the namespace clash.  I kept "make sed" as a phony target for backward
> compatibility, but personally I would prefer to remove them.
>
> I'll respond in detail to the issues below after I send it out ... I
> can definitely change things based on your feedback, but it might have
> to be on top of this patch... patch #5 on top of #4 to address your
> feedback.
>
> Andy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Build-and-test-changes-to-support-Clang-sanitizers.patch
Type: text/x-patch
Size: 18076 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20160320/0943c537/attachment-0005.bin>