Closed
Bug 1264692
Opened 9 years ago
Closed 8 years ago
[rep tests] Add tests for object-with-url rep
Categories
(DevTools :: Shared Components, enhancement, P1)
DevTools
Shared Components
Tracking
(firefox49 fixed)
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: linclark, Assigned: linclark)
References
Details
(Whiteboard: [devtools-html])
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
linclark
:
review+
|
Details | Diff | Splinter Review |
See Bug 1257552
Updated•9 years ago
|
Severity: normal → enhancement
Whiteboard: [devtools-html]
Updated•9 years ago
|
Blocks: devtools-html-2
Updated•8 years ago
|
Flags: qe-verify-
Priority: -- → P1
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → lclark
Updated•8 years ago
|
Status: NEW → ASSIGNED
Iteration: --- → 49.1 - May 9
Assignee | ||
Comment 1•8 years ago
|
||
:Honza, when I run the attached test I get Rep correctly selects ObjectWithURL - got "UndefinedRep", expected "ObjectWithURL". Do you know why this would be happening for that stub grip?
Flags: needinfo?(odvarko)
Comment 2•8 years ago
|
||
(In reply to Lin Clark [:linclark] from comment #1)
> Created attachment 8747682 [details] [diff] [review]
> Bug1264692.patch
>
> :Honza, when I run the attached test I get Rep correctly selects
> ObjectWithURL - got "UndefinedRep", expected "ObjectWithURL". Do you know
> why this would be happening for that stub grip?
There are two things causing the failure:
1) The grip (or any other object) must be passed as 'object' prop into a rep.
const renderedRep = shallowRenderComponent(Rep, {object: gripStub});
2) The gripStub in the test has prop "class" set to "Window", which causes the Window rep to be picked. You need to remove it (in order to get the "ObjectWithURL").
Honza
Flags: needinfo?(odvarko)
Comment 3•8 years ago
|
||
Forgot to add. The test in bug 1264700 should also be fixed since the grip is not passed in as 'object' props and the result is undefined (which is correct, by by coincidence).
Honza
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8748283 -
Flags: review?(odvarko)
Assignee | ||
Comment 5•8 years ago
|
||
Here's the test, including the TODO to test the link once it's implmented.
I also removed the check for the grip.preview.kind property in getDescription since it's already checked in supportsObject, but I can revert that if you like.
Attachment #8747682 -
Attachment is obsolete: true
Attachment #8748283 -
Attachment is obsolete: true
Attachment #8748283 -
Flags: review?(odvarko)
Attachment #8748285 -
Flags: review?(odvarko)
Comment 6•8 years ago
|
||
Comment on attachment 8748285 [details] [diff] [review]
Bug1264692.patch
Review of attachment 8748285 [details] [diff] [review]:
-----------------------------------------------------------------
Please fix the URL. The test passes if the URL is correct and I don't see any other issues so, I am r+ it.
Honza
::: devtools/client/shared/components/reps/object-with-url.js
@@ +36,1 @@
> },
I like this change
::: devtools/client/shared/components/test/mochitest/test_reps_object-with-url.html
@@ +42,5 @@
> + // Test rendering
> + const renderedComponent = renderComponent(ObjectWithURL.rep, { object: gripStub });
> + ok(renderedComponent.className.includes("objectLink-Location"), "ObjectWithURL rep has expected class name");
> + const innerNode = renderedComponent.querySelector(".objectPropValue");
> + is(innerNode.textContent, "about:privatebrowsing", "ObjectWithURL rep has expected inner HTML structure and text content");
The URL is wrong and the test fails on it.
Attachment #8748285 -
Flags: review?(odvarko) → review+
Assignee | ||
Comment 7•8 years ago
|
||
Thanks for catching that!
Attachment #8748285 -
Attachment is obsolete: true
Attachment #8748953 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Keywords: checkin-needed
Comment 9•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•