Closed Bug 1264692 Opened 4 years ago Closed 4 years ago

[rep tests] Add tests for object-with-url rep

Categories

(DevTools :: Shared Components, enhancement, P1)

enhancement

Tracking

(firefox49 fixed)

RESOLVED FIXED
Firefox 49
Iteration:
49.1 - May 9
Tracking Status
firefox49 --- 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: -- → P1
Assignee: nobody → lclark
Status: NEW → ASSIGNED
Iteration: --- → 49.1 - May 9
Attached patch Bug1264692.patch (obsolete) — Splinter Review
: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)
(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)
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
Attached patch Bug1264692.patch (obsolete) — Splinter Review
Attachment #8748283 - Flags: review?(odvarko)
Attached patch Bug1264692.patch (obsolete) — Splinter Review
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 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+
Attached patch Bug1264692.patchSplinter Review
Thanks for catching that!
Attachment #8748285 - Attachment is obsolete: true
Attachment #8748953 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7fa2a19a9c53
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.