[rep tests] Add tests for function rep

RESOLVED FIXED in Firefox 50

Status

()

Firefox
Developer Tools: Shared Components
P1
enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: linclark, Assigned: linclark)

Tracking

unspecified
Firefox 50
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: [devtools-html])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

2 years ago
See Bug 1257552
(Assignee)

Updated

2 years ago
Blocks: 1257552

Updated

2 years ago
Severity: normal → enhancement
Whiteboard: [devtools-html]

Updated

2 years ago
Blocks: 1263741

Updated

2 years ago
Flags: qe-verify-
Priority: -- → P2
(Assignee)

Updated

2 years ago
Assignee: nobody → lclark
(Assignee)

Comment 1

2 years ago
Created attachment 8764578 [details] [diff] [review]
Bug1264683.patch

I'm planning to consolidate the restRenderingInMode between the grips test and this one, but just posting a patch without that consolidation for now.
(Assignee)

Comment 2

2 years ago
Created attachment 8764610 [details] [diff] [review]
Bug1264683.patch
Attachment #8764578 - Attachment is obsolete: true
Attachment #8764610 - Flags: review?(odvarko)

Updated

2 years ago
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+
(Assignee)

Comment 4

2 years ago
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)
(Assignee)

Comment 5

2 years ago
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)
(Assignee)

Comment 6

2 years ago
Created attachment 8765464 [details] [diff] [review]
Bug1264683.patch

Rebased to resolve collision in chrome.ini
Attachment #8764610 - Attachment is obsolete: true
Attachment #8765464 - Flags: review+
(Assignee)

Updated

2 years ago
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
(Assignee)

Comment 8

2 years ago
Created attachment 8766332 [details] [diff] [review]
Bug1264683.patch

Rebased again, so hopefully works now
Attachment #8765464 - Attachment is obsolete: true
Flags: needinfo?(lclark)

Comment 9

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/c2cfdd905fdc
[rep tests] Add tests for function rep. r=Honza

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c2cfdd905fdc
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
You need to log in before you can comment on or make changes to this bug.