Closed
Bug 960421
Opened 12 years ago
Closed 11 years ago
Mozmill doesn't kill the running application if KeyboardInterrupt error is raised
Categories
(Testing Graveyard :: Mozmill, defect)
Tracking
(Not tracked)
RESOLVED
WORKSFORME
People
(Reporter: whimboo, Assigned: whimboo)
References
()
Details
(Whiteboard: [mozmill-2.1+])
Attachments
(1 file, 2 obsolete files)
598 bytes,
patch
|
whimboo
:
review-
|
Details | Diff | Splinter Review |
I noticed this a lot the last days. Whenever I hit Ctrl-C during a testrun the build is not getting shutdown. This can cause Firefox builds to remain open and blocking the machine from running further testruns.
Comment 1•12 years ago
|
||
I can't reproduce this on OSX or on Linux.
This is the case on Windows, but I think the problem there lies with the terminal we are using.
Assignee | ||
Updated•12 years ago
|
Whiteboard: [mozmill-2.0.4+] → [mozmill-2.0.4+][mentor=whimboo][lang=py]
Assignee | ||
Updated•11 years ago
|
Comment 3•11 years ago
|
||
I could reproduce this on OSX.
The normal tests under js/manifest.ini work as expected. But, the restart tests under js/testFrame/testRestart/manifest.ini do not show the desired result. While running the restart tests, when I hit Ctrl+C, the cli exits with a 'TEST-UNEXPECTED-FAIL' error but, the firefox build remains open.
Currently, I'm working to fix this bug.
Assignee | ||
Comment 4•11 years ago
|
||
Hi Sambuddha! It's great that you want to work on this bug. It's a real blocker for us for the next Mozmill release. So I'm happy to give you any help you need to get this issue fixed. As already said on IRC you had a good start here, that you were already able to reproduce the problem!
I have assigned the bug to you now.
Assignee: nobody → sammygamer
Status: NEW → ASSIGNED
Comment 5•11 years ago
|
||
Attachment #8366602 -
Flags: review?(hskupin)
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 8366602 [details] [diff] [review]
This is the fix for the bug.
Review of attachment 8366602 [details] [diff] [review]:
-----------------------------------------------------------------
Applying the patch fails due to white-space issues. Please see details inline.
::: mozmill/mozmill/__init__.py
@@ +415,4 @@
>
> self.runner.reset()
>
> + except (JSBridgeDisconnectError, KeyboardInterrupt), e:
There are still white-space issues. In line #417 a lot of blanks are left, while in line #418 except has been pushed one position to the left.
@@ +419,4 @@
> frame = None
> self.handle_disconnect()
> + if isinstance(e, KeyboardInterrupt):
> + break
Same here. You might want to disable using tab characters. We use spaces for indentation, and for Python there should be 4 of them.
Attachment #8366602 -
Flags: review?(hskupin) → review-
Comment 7•11 years ago
|
||
Attachment #8366602 -
Attachment is obsolete: true
Attachment #8366608 -
Flags: review?(hskupin)
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 8366608 [details] [diff] [review]
Indentation problem fixed
Review of attachment 8366608 [details] [diff] [review]:
-----------------------------------------------------------------
Even with this patch applied it doesn't work for me when I run the tests in mutt/mutt/tests/js/testRestart/manifest.ini. When I wait until Firefox has been restarted the first time and reopened again, hit ctrl-c the CLI will exit but Firefox is still running.
So i think that the break doesn't help us that much here. Most likely because Firefox gets restarted by itself, and hitting ctrl-c will exit the loop but not kill the newly spawned process.
Attachment #8366608 -
Flags: review?(hskupin) → review-
Comment 9•11 years ago
|
||
Attachment #8366608 -
Attachment is obsolete: true
Attachment #8366646 -
Flags: review?(hskupin)
Assignee | ||
Comment 10•11 years ago
|
||
Given that we have to release Mozmill 2.0.4 as soon as possible, I'm going to move this bug for the 2.0.5 release. Reason is that it only affects testruns canceled by the user, and it wont help us in mozmill-ci.
Assignee | ||
Comment 11•11 years ago
|
||
Given that the 2.0.4 release was busted we have to release 2.0.5 for only bug fixes. I would like to push this bug out to the next feature release.
Whiteboard: [mozmill-2.0.5+][mentor=whimboo][lang=py] → [mozmill-2.1?][mentor=whimboo][lang=py]
Assignee | ||
Comment 12•11 years ago
|
||
Comment on attachment 8366646 [details] [diff] [review]
Updated bug fix
Review of attachment 8366646 [details] [diff] [review]:
-----------------------------------------------------------------
I finally was able to test this patch on my OS X box. A testrun for restart tests have shown me that we successfully quit the testrun, but I get a different exception now, which I don't expect to see:
Traceback (most recent call last):
File "/Volumes/data/code/mozmill/mozmill/mozmill/__init__.py", line 795, in run
mozmill.run(tests, self.options.restart)
File "/Volumes/data/code/mozmill/mozmill/mozmill/__init__.py", line 421, in run
self.handle_disconnect()
File "/Volumes/data/code/mozmill/mozmill/mozmill/__init__.py", line 479, in handle_disconnect
self.stop_runner()
File "/Volumes/data/code/mozmill/mozmill/mozmill/__init__.py", line 525, in stop_runner
frame = jsbridge.JSObject(self.bridge, js_module_frame)
File "/Volumes/data/code/mozmill/jsbridge/jsbridge/jsobjects.py", line 73, in __init__
name = bridge.set(name)['data']
AttributeError: 'NoneType' object has no attribute 'set'
In such a case we should not try to call stop_runner which will send additional information over the bridge. Maybe this bug has to wait until I was able to fix bug 794020, which will add a hard-stop method.
Attachment #8366646 -
Flags: review?(hskupin) → review-
Updated•11 years ago
|
Whiteboard: [mozmill-2.1?][mentor=whimboo][lang=py] → [mozmill-2.1?][mentor=whimboo][lang=py][good first bug]
Updated•11 years ago
|
Mentor: hskupin
Whiteboard: [mozmill-2.1?][mentor=whimboo][lang=py][good first bug] → [mozmill-2.1?][lang=py][good first bug]
Comment 13•11 years ago
|
||
Is work still being done on this bug? if not, lets unassign it.
Flags: needinfo?(hskupin)
Assignee | ||
Comment 14•11 years ago
|
||
We are currently blocked on bug 794020. I hope to finally get to it soon, so we can continue here.
Flags: needinfo?(hskupin)
Assignee | ||
Updated•11 years ago
|
Assignee: sambuddhabasu1 → hskupin
Whiteboard: [mozmill-2.1?][lang=py][good first bug] → [mozmill-2.1+]
Assignee | ||
Comment 15•11 years ago
|
||
I re-tested this but I can no longer reproduce it. Hitting Ctrl+C while the restart tests are running successfully closes the application. It even works during a restart of the application, and across all platforms.
There is nothing we can do here, so marking as WFM.
Mentor: hskupin
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
Updated•9 years ago
|
Product: Testing → Testing Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•