Closed Bug 1264683 Opened 4 years ago Closed 3 years ago

[rep tests] Add tests for function rep

Categories

(DevTools :: Shared Components, enhancement, P1)

enhancement

Tracking

(firefox50 fixed)

RESOLVED FIXED
Firefox 50
Iteration:
50.2 - Jul 4
Tracking Status
firefox50 --- fixed

People

(Reporter: linclark, Assigned: linclark)

References

Details

(Whiteboard: [devtools-html])

Attachments

(1 file, 3 obsolete files)

See Bug 1257552
Blocks: 1257552
Severity: normal → enhancement
Whiteboard: [devtools-html]
Flags: qe-verify-
Priority: -- → P2
Assignee: nobody → lclark
Attached patch Bug1264683.patch (obsolete) — Splinter Review
I'm planning to consolidate the restRenderingInMode between the grips test and this one, but just posting a patch without that consolidation for now.
Attached patch Bug1264683.patch (obsolete) — Splinter Review
Attachment #8764578 - Attachment is obsolete: true
Attachment #8764610 - Flags: review?(odvarko)
Status: NEW → ASSIGNED
Iteration: --- → 50.2
Priority: P2 → P1
Comment on attachment 8764610 [details] [diff] [review]
Bug1264683.patch

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

Just one comment, please fix. Otherwise looks good and it's passing for me. R+

Thanks!

Honza

::: devtools/client/shared/components/test/mochitest/test_reps_function.html
@@ +110,5 @@
> +          "extensible": true,
> +          "frozen": false,
> +          "sealed": false,
> +          "name": "testName",
> +          "displayName": "testName",

Note that `displayName` has higher priority over `name` and should be always used if available (e.g. also in stack frames etc.). You could change the `name` value here to something else than `testName` to test it.

Honza
Attachment #8764610 - Flags: review?(odvarko) → review+
Do you know in what cases you would get a grip that has a different name and display name?

When I do this in the console, it returns a grip with the same name/displayName:

>let foo = function testName(){ let innerVar = "foo" }
>foo
Flags: needinfo?(odvarko)
Discussed with Honza in the meeting. It turns out the displayName can be manually altered, but in our grips we use userDisplayName to hold this value, not displayName. Added Bug 1282427 to handle this. For now, the test will be committed as is.
Flags: needinfo?(odvarko)
Attached patch Bug1264683.patch (obsolete) — Splinter Review
Rebased to resolve collision in chrome.ini
Attachment #8764610 - Attachment is obsolete: true
Attachment #8765464 - Flags: review+
Keywords: checkin-needed
Hi Lin, 

this seems to have problems again

Hunk #3 FAILED at 90
Hunk #4 FAILED at 120
2 out of 6 hunks FAILED -- saving rejects to file devtools/client/shared/components/test/mochitest/test_reps_grip.html.rej

maybe because of https://bugzilla.mozilla.org/show_bug.cgi?id=1281489
Flags: needinfo?(lclark)
Keywords: checkin-needed
Attached patch Bug1264683.patchSplinter Review
Rebased again, so hopefully works now
Attachment #8765464 - Attachment is obsolete: true
Flags: needinfo?(lclark)
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/c2cfdd905fdc
[rep tests] Add tests for function rep. r=Honza
https://hg.mozilla.org/mozilla-central/rev/c2cfdd905fdc
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.