Closed Bug 967759 Opened 7 years ago Closed 2 years ago

Remove unnecessary eval calls from frontend tests

Categories

(DevTools :: Debugger, defect, P3)

x86_64
Linux
defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: jimb, Unassigned)

References

(Blocks 1 open bug)

Details

In bug 322176, we made the line numbers reported for eval more reasonable. However, it runs afoul of the many debugger front-end mochitests that have text like:

function firstCall() {
  eval("secondCall();");
}

...

function secondCall() {
  // This comment is useful: ☺
  eval("debugger;");
  function foo() {}
  if (true) {
    foo();
  }
}

Before bug 332176, the eval code would be attributed to the line containing the eval call. Post-patch, the eval code is attributed to line 1 --- of the *evaluated* code. So all those mochitests had their expected line numbers changed from something like "6" or "18" to, simply, 1.

It turns out that those 'eval' calls are not essential to the tests. They predate the implementation of Debugger.prototype.findScripts, to a time when onNewScript notifications were the only way for the debugger to see a script --- so the tests used 'eval' to ensure that those notifications were generated. But we don't need them any more.

Since the 'eval' calls effectively erase meaningful information --- at what location were we executing? --- they should be removed from the debuggee code, and the original, meaningful line numbers reinstated.

(Kannan volunteered on IRC to take this on.)
Status: NEW → ASSIGNED
Priority: -- → P3
Kannan, are you still volunteering for this?
Flags: needinfo?(kvijayan)
Summary: Debugger front end mochitests should lose gratuitous 'eval' calls → Remove unnecessary eval calls from frontend tests
Not volunteering for this anymore.  My plate is pretty full.  Trying to clear needinfos.
Flags: needinfo?(kvijayan)
Thanks for the update Kannan!
Assignee: kvijayan → nobody
Blocks: dbg-test
Bulk changing the status, as there is no assignee anymore.

Sebastian
Status: ASSIGNED → NEW
Product: Firefox → DevTools

closing as we have re-written the mochitests

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.