Closed Bug 654354 Opened 11 years ago Closed 11 years ago

Mochitest spews two stacktraces when killed with ctrl-c, but should die quietly.

Categories

(Testing :: Mochitest, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)

Details

Attachments

(1 file, 1 obsolete file)

Patch in a moment.
Attached patch Patch v1 (obsolete) — Splinter Review
Attachment #529619 - Flags: review?(ted.mielczarek)
Assignee: nobody → justin.lebar+bug
Comment on attachment 529619 [details] [diff] [review]
Patch v1

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

::: testing/mochitest/pywebsocket/standalone.py
@@ +461,5 @@
>                                   WebSocketRequestHandler)
>          server.serve_forever()
> +    except KeyboardInterrupt, e:
> +        # Exit quietly when we're killed by ctrl-c.
> +        sys.exit(1)

pywebsocket is an upstream project, this should really go there otherwise we run the risk of losing your customization if we import a new version. I guess this isn't something you can hack into your pywebsocket_ignore_sigint.py wrapper?
Attachment #529619 - Flags: review?(ted.mielczarek) → review+
Ack, I forgot that pywebsocket is upstream.

I'll see if they'll take a patch.
> I guess this isn't something you can hack into your 
> pywebsocket_ignore_sigint.py wrapper?

Correct; I think it has to wrap around the logging.critical(str(e)).

pywebsocket appears to die a bit after I press ctrl-c; this is annoying since I get my prompt back, start typing, and *then* the backtrace spews.
We could just silence all pywebsocket output, but that seems prone to hide errors in the future.
Indeed; that would have made it very hard to notice the issue we had with pywebsocket dying on ctrl-c under interactive debuggers.
> I guess this isn't something you can hack into your 
> pywebsocket_ignore_sigint.py wrapper?

Actually, now that I think about it...maybe!
Attached patch Patch v2Splinter Review
This patch works with no changes to upstream pywebsocket.
Attachment #529619 - Attachment is obsolete: true
Attachment #532340 - Flags: review?(ted.mielczarek)
Comment on attachment 532340 [details] [diff] [review]
Patch v2

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

Good call!

::: testing/mochitest/runtests.py
@@ +374,5 @@
>      self._scriptdir = scriptdir
>      self.debuggerInfo = debuggerInfo
>  
>    def start(self):
> +    # Invoke pywebscocket through a wrapper which adds special SIGINT handling.

typoed pywebsocket.
Attachment #532340 - Flags: review?(ted.mielczarek) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.