Adapt test framework to function toString revision (Bug 1317400)

RESOLVED FIXED in Firefox 54

Status

()

Toolkit
WebExtensions: General
P1
normal
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: zombie, Assigned: zombie)

Tracking

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

9 months ago
See bug 1317400, in short:

object methods' toString produces code that can't be evaluated directly.  So instead of changing all of our tests [1], we should just adapt our test framework.

1) https://bugzilla.mozilla.org/attachment.cgi?id=8841177&action=diff
(Assignee)

Updated

9 months ago
Assignee: nobody → tomica
Blocks: 1317400
Status: NEW → ASSIGNED
Priority: -- → P1
Comment hidden (mozreview-request)
(Assignee)

Comment 2

9 months ago
mozreview-review
Comment on attachment 8842627 [details]
Bug 1343583 - Adapt test framework to function toString revision

https://reviewboard.mozilla.org/r/116400/#review118028

::: toolkit/components/extensions/ExtensionTestCommon.jsm:283
(Diff revision 1)
>            zipW.addEntryDirectory(path, time, false);
>          }
>        }
>      }
>  
> +    function serialize(code) {

I'm guessing we don't have an easy way to share this between the test frameworks, or otherwise we wouldn't be duplicating all this?
Comment hidden (mozreview-request)
(Assignee)

Comment 4

9 months ago
mozreview-review
Comment on attachment 8842645 [details]
Bug 1343583 - Fix individual tests for function toString revision

https://reviewboard.mozilla.org/r/116408/#review118046

::: browser/components/extensions/test/browser/browser_ext_browserAction_context.js:178
(Diff revision 1)
>        "default-2.png": imageBuffer,
>        "1.png": imageBuffer,
>        "2.png": imageBuffer,
>      },
>  
> -    getTests(tabs, expectDefaults) {
> +    getTests: function(tabs, expectDefaults) {

All these don't go though the test framework, so need to be fixed by hand.
(Assignee)

Updated

9 months ago
Attachment #8842627 - Flags: review?(aswan)
Attachment #8842645 - Flags: review?(aswan)

Comment 5

9 months ago
mozreview-review
Comment on attachment 8842627 [details]
Bug 1343583 - Adapt test framework to function toString revision

https://reviewboard.mozilla.org/r/116400/#review118080

::: testing/mochitest/tests/SimpleTest/ExtensionTestUtils.js:90
(Diff revision 1)
>  
>    // Mimic serialization of functions as done in `Extension.generateXPI` and
>    // `Extension.generateZipFile` because functions are dropped when `ext` object
>    // is sent to the main process via the message manager.
> +
> +  function serialize(code) {

nit: can you name this serializeScript or something just to be clear what it does
Attachment #8842627 - Flags: review?(aswan) → review+

Comment 6

9 months ago
mozreview-review
Comment on attachment 8842645 [details]
Bug 1343583 - Fix individual tests for function toString revision

https://reviewboard.mozilla.org/r/116408/#review118084
Attachment #8842645 - Flags: review?(aswan) → review+
Comment on attachment 8842627 [details]
Bug 1343583 - Adapt test framework to function toString revision

https://reviewboard.mozilla.org/r/116400/#review118028

> I'm guessing we don't have an easy way to share this between the test frameworks, or otherwise we wouldn't be duplicating all this?

We do. Just add it to ExtensionTestCommon and import that from the other places you need to use it.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Keywords: checkin-needed

Comment 12

9 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3b9e06e3d9b8
Adapt test framework to function toString revision r=aswan
https://hg.mozilla.org/integration/autoland/rev/10fccfc11db1
Fix individual tests for function toString revision r=aswan
Keywords: checkin-needed
Backed out in https://hg.mozilla.org/integration/autoland/rev/ccbea1069d10 at :zombie's request.
Flags: needinfo?(tomica)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Flags: needinfo?(tomica)
Keywords: checkin-needed

Comment 16

9 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b93c92e29ed6
Adapt test framework to function toString revision r=aswan
https://hg.mozilla.org/integration/autoland/rev/b46002a33c2c
Fix individual tests for function toString revision r=aswan
Keywords: checkin-needed

Comment 17

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b93c92e29ed6
https://hg.mozilla.org/mozilla-central/rev/b46002a33c2c
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.