Closed Bug 1090929 Opened 5 years ago Closed 5 years ago

Enable the debugger tests for e10s

Categories

(DevTools :: Debugger, defect)

x86
macOS
defect
Not set

Tracking

(e10s+)

RESOLVED FIXED
Firefox 36
Tracking Status
e10s + ---

People

(Reporter: ejpbruel, Assigned: ejpbruel)

References

Details

Attachments

(16 files)

26.22 KB, patch
miker
: review+
Details | Diff | Splinter Review
10.78 KB, patch
miker
: review+
Details | Diff | Splinter Review
22.81 KB, patch
miker
: review+
Details | Diff | Splinter Review
9.21 KB, patch
miker
: review+
Details | Diff | Splinter Review
26.98 KB, patch
miker
: review+
Details | Diff | Splinter Review
12.97 KB, patch
miker
: review+
Details | Diff | Splinter Review
7.04 KB, patch
miker
: review+
Details | Diff | Splinter Review
14.67 KB, patch
miker
: review+
Details | Diff | Splinter Review
15.47 KB, patch
miker
: review+
Details | Diff | Splinter Review
1.34 KB, patch
miker
: review+
Details | Diff | Splinter Review
3.35 KB, patch
miker
: review+
Details | Diff | Splinter Review
24.44 KB, patch
miker
: review+
Details | Diff | Splinter Review
35.46 KB, patch
miker
: review+
Details | Diff | Splinter Review
11.44 KB, patch
miker
: review+
Details | Diff | Splinter Review
84.51 KB, patch
miker
: review+
Details | Diff | Splinter Review
47.13 KB, patch
fitzgen
: review+
Details | Diff | Splinter Review
No description provided.
Attachment #8513445 - Flags: review?(mratcliffe)
Attachment #8513445 - Flags: review?(mratcliffe) → review+
Attachment #8514142 - Flags: review?(mratcliffe)
Green try push for the blackboxing tests:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ebe1cafbed90
Green try push for the break-on-dom tests:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d51d315e091f
I just realized those try pushes are probably invalid because I didn't enable the e10s tests on try (I had a commit that does that, but didn't include it in the diff I used to push to try).

The blackboxing tests already landed, but I'm confident those won't cause any problems. I will redo the try push for the break-on-dom tests. If there are any problems with the blackboxing tests, they should show up in that try push as well.
Attachment #8514267 - Flags: review?(mratcliffe)
Attachment #8514350 - Flags: review?(mratcliffe)
Attachment #8514436 - Flags: review?(mratcliffe)
Attachment #8514444 - Flags: review?(mratcliffe)
Attachment #8514449 - Flags: review?(mratcliffe)
Attachment #8514468 - Flags: review?(mratcliffe)
Attachment #8514142 - Flags: review?(mratcliffe) → review+
Attachment #8514267 - Flags: review?(mratcliffe) → review+
Attachment #8514350 - Flags: review?(mratcliffe) → review+
Attachment #8514436 - Flags: review?(mratcliffe) → review+
Attachment #8514444 - Flags: review?(mratcliffe) → review+
Attachment #8514449 - Flags: review?(mratcliffe) → review+
Attachment #8514468 - Flags: review?(mratcliffe) → review+
Assignee: nobody → ejpbruel
Depends on: 1093535
Green try run for the break-on-dom tests:
https://hg.mozilla.org/try/rev/ca60639afdd6
Attachment #8518077 - Flags: review?(mratcliffe)
Attachment #8518078 - Flags: review?(mratcliffe)
Attachment #8518085 - Flags: review?(mratcliffe)
Attachment #8518087 - Flags: review?(mratcliffe)
Attachment #8518089 - Flags: review?(mratcliffe)
Attachment #8518089 - Attachment is patch: true
Attachment #8518089 - Attachment mime type: text/x-patch → text/plain
Attachment #8518090 - Flags: review?(mratcliffe)
Attachment #8518091 - Flags: review?(mratcliffe)
Once these patches land, about 90% of the frontend tests for the debugger should work on opt builds with E10S enabled.

On debug builds, the frontend tests are still leaking memory. Because I suspect the underlying cause is the same for each test, I've opened up a separate bug for this (1094749).
Depends on: 1094749
Attachment #8518077 - Flags: review?(mratcliffe) → review+
Attachment #8518078 - Flags: review?(mratcliffe) → review+
Attachment #8518085 - Flags: review?(mratcliffe) → review+
Attachment #8518087 - Flags: review?(mratcliffe) → review+
Attachment #8518089 - Flags: review?(mratcliffe) → review+
Comment on attachment 8518090 [details] [diff] [review]
Enable the stack tests

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

::: browser/devtools/debugger/test/browser.ini
@@ +412,2 @@
>  [browser_dbg_stack-03.js]
> +skip-if = e10s # TODO

Maybe add a bug number here.
Attachment #8518090 - Flags: review?(mratcliffe) → review+
Comment on attachment 8518091 [details] [diff] [review]
Enable the remaining tests

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

gTab is sometimes not cleaned up, r+ if you fix it.

The TODO comments should probably have a bug number or explaination.

::: browser/devtools/debugger/test/browser_dbg_event-listeners-01.js
@@ +23,5 @@
>        "Root actor should identify itself as a browser.");
>  
>      addTab(TAB_URL)
> +      .then((aTab) => {
> +        gTab = aTab;

gTab is not cleaned up anywhere... not sure why this doesn't leak but it should be cleaned up anyhow.

::: browser/devtools/debugger/test/browser_dbg_event-listeners-02.js
@@ +24,5 @@
>        "Root actor should identify itself as a browser.");
>  
>      addTab(TAB_URL)
> +      .then((aTab) => {
> +        gTab = aTab;

Same again

::: browser/devtools/debugger/test/browser_dbg_event-listeners-03.js
@@ +24,5 @@
>        "Root actor should identify itself as a browser.");
>  
>      addTab(TAB_URL)
> +      .then((aTab) => {
> +        gTab = aTab;

Again.
Attachment #8518091 - Flags: review?(mratcliffe) → review+
Failing try run for the source-mapping tests:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=33232f025e27
Based on the try log, the failing seems to be caused by bug 1093535. Strangely, I cannot reproduce the issue on try. I've disabled the failing test for now and did another push to try.
Green try run for the source-mapping tests:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=a9478a8cf99d
Green try push for the variables-view-popup tests:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8d2987c65d21
Green try run for the variables-view-filter tests:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=fec0804947b7
Green try run for the variables-view tests:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=a70cdb007874
Green try run for the conditional-breakpoint tests:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8241651b21d9
I just noticed that I was forgot to ask r+ for some of the variables-view tests, sorry about that Mike!
Attachment #8521382 - Flags: review?(mratcliffe)
Green try run for the variables-view-* tests:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=47062604de60
Comment on attachment 8521382 [details] [diff] [review]
Enable the variables-view-* tests

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

::: browser/devtools/debugger/test/head.js
@@ +970,5 @@
>    waitForMessageFromTab(tab, "test:call");
>  }
>  
> +function evalInTab(tab, string) {
> +  info("Evalling string " + string + " in tab.");

Might want to limit this dump to first n chars on first line of code to be eval'd, or else the logs might get crazy noisy for multi-line code evals.
Attachment #8521382 - Flags: review?(mratcliffe) → review+
Seeing test failures for the search tests on try (confusingly, in other tests):
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=020de4c6498c
Some try failures for the remaining tests:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e8d23e5ad3ab
Green try run for the remaining tests after I redisabled two of them:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6580af59fbda
With that patch, 203 out of 223 debugger tests are now running on Linux opt builds with e10s enabled. The remaining 20 tests are either blocked on bug 1093535 or have miscellaneous other issues. Finally, enabling these tests on Linux debug builds as well is blocked on bug 1073352.

Once the above patch sticks, this bug can be resolved. I'll open new bugs for enabling the remaining debugger tests for e10s and enabling the debugger tests for e10s debug builds.
Depends on: 1073352
No longer depends on: 1094749
Whiteboard: [leave-open]
Enable the debugger tests for e10s debug builds:
https://bugzilla.mozilla.org/show_bug.cgi?id=1103839
Enable the remaining debugger tests for e10s opt builds:
https://bugzilla.mozilla.org/show_bug.cgi?id=1103841
https://hg.mozilla.org/mozilla-central/rev/26112e1b5c61
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.