[Toybox] [PATCH] killall.test, pidof.test: deflake.

[Rob Landley]

On 9/19/19 9:45 AM, enh wrote:
> On Thu, Sep 19, 2019 at 7:37 AM Rob Landley <rob at landley.net> wrote:
>>
>> On 9/16/19 3:17 PM, enh via Toybox wrote:
>>> Rarely (but these tests now get run often), it seems like we catch the
>>> shell between its fork and exec of sleep, which counts as false
>>> positives for killall/pidof. Since we don't actually need to sleep, just
>>> have the shell script spin instead.
>>
>> What's the failure here? The fork but not yet exec means we think the shell has
>> done a (subshell) or backgrounded& something, so we killed two instances of the
>> test. Ok. This is bad how? Isn't killall supposed to kill all? (What test
>> actually fails?)

I try not to have cpu-eating loops in the test suite because if the test fails
(or is interrupted by Ctrl-C) it potentially leaves the cpu-eating loop running
forever, and on UP machines (QEMU) this can make later tests outright crotchety,
and when it does it on my laptop I may not notice until I'm asking "why has my
battery drained so fast".

That's why I added the sleep. Ideally I would have a container or qemu instance
to run these tests in so such debris is less of an issue...

> the test is checking the list of pids (so we can tell it's actually
> doing anything), so there's an unexpected extra entry.

Where is the test doing that?

  tst=toybox.test
  ./$tst &
  testing "short name" "killall -w $tst && echo killed ; \
    pgrep -l $tst || echo really" "killed\nreally\n" "" ""

killall isn't outputting the list of processes it killed, and pgrep -l is
checking to see if processes are left. Hmmm...

Ok, possibly the failure mode is the shell forks between the signal being sent
and the signal being received, so killall only killed one, so pgrep is finding
one that hasn't execed to become sleep yet. So it's not comparing the list of
processes killed it's the "have we really killed" test failing.

Hmmm... Ok, I added a sleep 0.1 which should be plenty of time for the exec to
happen? The loop isn't going to spawn _more_ of them. "help sleep" isn't coming
up with a shell builtin and if it did it wouldn't need to fork()...

If that doesn't fix it, I can yank the sleep out of the loop...

Rob