1437712 - Fix timeout in devtools/client/debugger/test/mochitest/browser_dbg_break-on-next.js with the patch for bug 1193394

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=701a987d4fcddd24a4f67f08df6f9ee2b9b81f3a&selectedJob=161714381

https://treeherder.mozilla.org/logviewer.html#?job_id=161714381&repo=try&lineNumber=8371

Comment 1

[Hiroyuki Ikezoe (:hiro)]

The timeout happened where we wait for oncePaused promise.
CCing bgrinstead who introduce this test case in bug 789430.

[1] https://hg.mozilla.org/mozilla-central/file/6d8f470b2579/devtools/client/debugger/test/mochitest/browser_dbg_break-on-next.js

Comment 2

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)
> The timeout happened where we wait for oncePaused promise.
> CCing bgrinstead who introduce this test case in bug 789430.
> 
> [1]
> https://hg.mozilla.org/mozilla-central/file/6d8f470b2579/devtools/client/
> debugger/test/mochitest/browser_dbg_break-on-next.js

Correct link is;

https://hg.mozilla.org/mozilla-central/file/6d8f470b2579/devtools/client/debugger/test/mochitest/browser_dbg_break-on-next.js#l78

interruptedByResumeButton [1] is false in the failure case for some reasons.

[1] https://hg.mozilla.org/mozilla-central/file/6d8f470b2579/devtools/client/debugger/debugger-controller.js#l586

Comment 3

[Hiroyuki Ikezoe (:hiro)]

From the test;

    let onceResumed = gTarget.once("thread-resumed");
    EventUtils.sendMouseEvent({ type: "mousedown" }, gResumeButton, gDebugger);   // (1)
    yield onceResumed;
  });

  let testEvent = Task.async(function* () {
    info("Starting testEvent");

    let oncePaused = gTarget.once("thread-paused");
    EventUtils.sendMouseEvent({ type: "mousedown" }, gResumeButton, gDebugger);
    once(gDebugger.gClient, "willInterrupt").then(() => {
      generateMouseClickInTab(gTab, "content.document.querySelector('button')");
    });
    yield oncePaused; // (2)

What's going on the failure case is;

1. In the previous test we do dispatch 'mousedown' event at (1)
2. _onResumePressed() in toolbar-view.js is called
3. this.activeThread.resume(this.resumptionWarnFunc) _onResumePressed() in is called, but 'resumed' event has not processed at this moment
4. the next test starts
5. the 'resumed' event is processed while we are waiting for 'thread-paused' Promise
6. _update() in debugger-controller.js is called
7. this.interruptedByResumeButton is set to false in _update() since the received event is 'resumed'

After 7, we don't have a chance to set interruptedByResumeButton to true.

I don't have any idea to solve this yet, but I think we should fix implementation instead of tweaking the test.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

One thing I am really wondering is why this failure hasn't appeared in the past tries?

Comment 5

[Brian Grinstead [:bgrins]]

This is for the old debugger UI which we don't ship anymore. Jason, is there an analogous test for the new frontend which would allow us to just delete / skip this one instead of fixing it?

Comment 6

[Jason Laster [:jlast]]

actually, i dont think we do, just created an issue https://github.com/devtools-html/debugger.html/issues/5446

the underlying logic is tested here https://searchfox.org/mozilla-central/source/devtools/server/tests/unit/test_interrupt.js so it is probably okay to skip and we'll add the new UI test soon.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Thank you both!  Let's skip it.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1437712 - Skip browser_dbg_break-on-next.js.

This test will cause timeout once we have a microtask checkpoint at the end of
a callback (bug 1193394).  Fortunately the test is for the old debugger UI that
we don't ship anymore, so we can just skip it.

Review commit: https://reviewboard.mozilla.org/r/221072/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/221072/

Comment 9

[Brian Grinstead [:bgrins]]

Comment on attachment 8951796 [details]
Bug 1437712 - Skip browser_dbg_break-on-next.js.

https://reviewboard.mozilla.org/r/221072/#review229744

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7cab5776a7d
Skip browser_dbg_break-on-next.js. r=bgrins

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/c7cab5776a7d

Comment 12

[Jason Laster [:jlast]]

Comment on attachment 8951796 [details]
Bug 1437712 - Skip browser_dbg_break-on-next.js.

https://reviewboard.mozilla.org/r/221072/#review230228