GNOME Bugzilla – Bug 742797
Errors thrown when tracking coverage can cause tests to hang
Last modified: 2015-01-18 21:40:32 UTC
A common pattern in some test cases is to start a main loop. The debugger object which keeps track of which lines are executing for the purposes of coverage counting may throw an exception. If it does, then the exception will be thrown in both compartments and the main loop will never exit. This causes tests to hang. Instead, we should handle the exception gracefully - e.g., log the error and refuse to count coverage for that file.
Created attachment 294327 [details] [review] Don't allow exceptions from debugger to propagate
Created attachment 294511 [details] [review] coverage: Don't allow exceptions to propagate in debugger compartment Slightly revised, fixed a warning about an implicit return undefined.
Review of attachment 294511 [details] [review]: ::: modules/coverage.js @@ +653,3 @@ + + this.deleteStatistics = function(filename) { + coveredFiles[filename] = undefined Missing semi. @@ +654,3 @@ + this.deleteStatistics = function(filename) { + coveredFiles[filename] = undefined + } Missing semi. @@ +710,3 @@ + frame.onStep = undefined + frame._branchTracker = undefined + deleteStatistics(frame.script.url) Lots of semis missing in all of this. @@ +758,3 @@ + * statistics for this file */ + _logExceptionAndReset(e, name, line); + return undefined; No need for this. @@ +761,3 @@ + } + + return undefined; ... or this, it's implicit.
Created attachment 294789 [details] [review] Don't propagate exceptions raised in debuggee (with style fixes) Implemented style fixes as suggested. Thanks for pointing those out, I think I've been spoiled by Python lately.
Created attachment 294790 [details] [review] Fix other found jshint errors Decided to run the file through jshint to see if there's any other similar mistakes (knowing me). Found a few that are probably just worth nipping in the bud now.