browser.test.* stacks are no longer useful

RESOLVED FIXED in Firefox 52

Status

RESOLVED FIXED
2 years ago
7 months ago

People

(Reporter: kmag, Assigned: kmag)

Tracking

51 Branch
mozilla52

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

2 years ago
They report the stack in the parent process, from the point where the message was received, rather than from where the assertion function was called.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 7

2 years ago
mozreview-review
Comment on attachment 8809685 [details]
Bug 1316780: Part 1 - Proxy extension events between the parent and child process.

https://reviewboard.mozilla.org/r/92220/#review92416
Attachment #8809685 - Flags: review?(aswan) → review+

Comment 8

2 years ago
mozreview-review
Comment on attachment 8809686 [details]
Bug 1316780: Part 2 - Remove ext-test.js.

https://reviewboard.mozilla.org/r/92222/#review92418

::: toolkit/components/extensions/ext-c-test.js:87
(Diff revision 2)
> +    extension.emit("test-result", Boolean(value), String(msg));
>    }
>  
>    return {
>      test: {
> -      // These functions accept arbitrary values. Convert the parameters to
> +      sendMessage: function(...args) {

nit: why this syntax instead of just `sendMessage(...args) {`
Attachment #8809686 - Flags: review?(aswan) → review+
(Assignee)

Comment 9

2 years ago
mozreview-review-reply
Comment on attachment 8809686 [details]
Bug 1316780: Part 2 - Remove ext-test.js.

https://reviewboard.mozilla.org/r/92222/#review92418

> nit: why this syntax instead of just `sendMessage(...args) {`

Just because I moved this code from ext-test.js and didn't change it. I was thinking about it, though, so I guess I will now.

Comment 10

2 years ago
mozreview-review
Comment on attachment 8809687 [details]
Bug 1316780: Part 3 - Capture the current unprivileged stack in browser.test.* assertion functions.

https://reviewboard.mozilla.org/r/92224/#review92420

Looks good to me and the SimpleTest change seems like a good one, but should probably be approved by an owner or peer for that code.

::: testing/mochitest/tests/SimpleTest/ExtensionTestUtils.js:57
(Diff revision 2)
>    }
>  
>    function testHandler(kind, pass, msg, ...args) {
>      if (kind == "test-eq") {
>        var [expected, actual] = args;
> -      SimpleTest.ok(pass, `${msg} - Expected: ${expected}, Actual: ${actual}`);
> +      SimpleTest.ok(pass, `${msg} - Expected: ${expected}, Actual: ${actual}`, undefined, args[2]);

can you just pull this out of args on the line above (and maybe switch var -> let at the same time)

::: toolkit/components/extensions/ext-c-test.js:104
(Diff revision 2)
>        notifyFail: function(msg) {
> -        extension.emit("test-done", false, msg);
> +        extension.emit("test-done", false, msg, getStack());
>        },
>  
>        log: function(msg) {
> -        extension.emit("test-log", true, msg);
> +        extension.emit("test-log", true, msg, getStack());

No need for a stack here...
Attachment #8809687 - Flags: review?(aswan) → review+
(Assignee)

Comment 11

2 years ago
mozreview-review-reply
Comment on attachment 8809687 [details]
Bug 1316780: Part 3 - Capture the current unprivileged stack in browser.test.* assertion functions.

https://reviewboard.mozilla.org/r/92224/#review92420

> No need for a stack here...

Yeah, I guess, but I figured it might be useful at some point so I may as well include it.
(Assignee)

Comment 12

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9867579cf623cd1396fee24d18179a149399e92
Bug 1316780: Part 1 - Proxy extension events between the parent and child process. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/c93fbd9ced3c64a4e48bb45ee90187be46f65212
Bug 1316780: Part 2 - Remove ext-test.js. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/ee3edae1e4e0ac07b3032a30b2685d2cd8c85658
Bug 1316780: Part 3 - Capture the current unprivileged stack in browser.test.* assertion functions. r=aswan

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b9867579cf62
https://hg.mozilla.org/mozilla-central/rev/c93fbd9ced3c
https://hg.mozilla.org/mozilla-central/rev/ee3edae1e4e0
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52

Updated

7 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.