[PATCH] D29546: [libFuzzer] Improve fuzzer-jobs test for Posix.

[Marcos Pividori via Phabricator via llvm-commits]

mpividori added a comment.

>> ` kill -0 ${FUZZER_PID};` only checks if the process if running and waits until it finishes with the while loop. But the condition could be false the first time you check. I mean, you are not checking if the process is still running after 2 seconds, you are only waiting.
>>  Because of that, I modified the test to:
>> 
>> - First check if the process is running: ``` kill -0 ${FUZZER_PID} ``` This retuns non-zero value if the process is not running.
> 
> Your description of what `kill -O ${FUZZER_PID}` is right but I think your change is wrong. If any `RUN:` lines fail then the test as a whole test will fail. That's how lit shell tests work. This means your change to the test will non-deterministically fail depending on when `LLVMFuzzer-EmptyTest` exits. We don't want that.

`LLVMFuzzer-EmptyTest` won't exit. It is an empty test. So, libFuzzer will always run for 4 seconds.

>> - Second, wait for the process to finish: ``` wait ${FUZZER_PID} ``` Instead of the while loop.
> 
> I didn't know about this shell built-in but again this change doesn't seem right to me. If `wait` is given a PID that has already finished then it will return a non-zero exit code and the test will fail. We don't want this.

Ok. I understood that the purpose of this test was to ensure that libFuzzer was running 2 jobs after 2 seconds. So I supposed that was the reason to run the process in background.

With my changes, the test also checks that the process is still running after 2 seconds. But now that I see the purpose of this test was different, I can just abandon this diff.

Thanks for your time.


https://reviews.llvm.org/D29546