runtests.py --testTimeOut=n does not work, runtests.py runs indefinitely

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
7 years ago
2 months ago

People

(Reporter: dschaffe, Unassigned)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
The --testTimeOut sets a limit in seconds to kill a test if it exceeds the limit.  We never got this working and we rely on buildbot to kill the run it exceeds 5m.  

In runtestBase.py we do:
p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
(out,err)=p.communicate()
- communicate blocks until the process finishes and causes runtests.py to hang if a test never ends

I think a better solution is to use polling with memory buffers and avoid communicate.
outFile=tempFile.spooledTemporaryFile()
errFile=tempFile.spooledTemporaryFile()
p = subprocess.Popen(cmd, stdout=outFile, stderr=errFile)
while True:
    if p.poll!=None:
        break
    if time()-start>self.testTimeOut:
        p.terminate()
        break
    sleep(.05) # switches to other threads
out=outFile.read()
err=errFile.read()
(Reporter)

Comment 2

7 years ago
Created attachment 569370 [details] [diff] [review]
patch
Attachment #569370 - Flags: review?(brbaker)
Attachment #569370 - Flags: feedback?(trbaker)

Updated

7 years ago
Attachment #569370 - Attachment is patch: true
(Reporter)

Comment 3

7 years ago
I think I see a minor speedup because we are not using communicate() and using the in memory temp file seems fast.

If timeout occurs the code tries to call terminate() to kill the process.  The code sleeps 1s, checks if the process is still running, and retries up to 10 times.  If the script cannot kill the process maybe an exception should be thrown and the script should stop.  I'm not sure if we should bail on the test run or just keep running registering a failure.  

I should make a minor change to the patch to gracefully handle the situation.  Currently the code will thrown an exception because p.poll() returns None.  Hopefully terminate() works consistently and we can always terminiate the process.

Comment 4

7 years ago
will this also work with run-acceptance-generic-adb.sh?  how about ssh?

Comment 5

7 years ago
Comment on attachment 569370 [details] [diff] [review]
patch

Review of attachment 569370 [details] [diff] [review]:
-----------------------------------------------------------------

Removing review request as this patch appears to have landed via bug 706071, tamarin-redux 6754:b442a94598f0
Attachment #569370 - Flags: review?(brbaker)

Updated

7 years ago
Attachment #569370 - Flags: feedback?(trbaker) → feedback+
Tamarin is a dead project now. Mass WONTFIX.
Status: NEW → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → WONTFIX
Tamarin isn't maintained anymore. WONTFIX remaining bugs.
You need to log in before you can comment on or make changes to this bug.