Closed
Bug 1317215
Opened 8 years ago
Closed 8 years ago
Add unit tests for LayoutActor and GridActor
Categories
(DevTools :: Inspector, defect, P3)
DevTools
Inspector
Tracking
(firefox55 fixed)
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: gl, Assigned: gl)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
No description provided.
Assignee | ||
Comment 1•8 years ago
|
||
Open to be picked up. We landed the LayoutActor and GridActor in bug 1308257. Note the comments on the current WIP patch in the last review in Bug 1308257:
::: devtools/server/tests/browser/browser_layout_getAllGrids.js
@@ +3,5 @@
> + http://creativecommons.org/publicdomain/zero/1.0/ */
> +
> +"use strict";
> +
> +// Check the output of getAllGrids
Please complete this comment a bit: "Check the output of getAllGrids when the document contains only one grid in the root window".
And then please add a couple more test cases: one where there is a grid in the root window and another one a child frame (and check the output of the method when passing true, and then false to the second parameter). And one where there are multiple grids in the root window, and check that the grid fragments is correct.
Assignee | ||
Updated•8 years ago
|
Priority: -- → P3
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → gl
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8810290 -
Attachment is obsolete: true
Comment 3•8 years ago
|
||
mozreview-review |
Comment on attachment 8859306 [details]
Bug 1317215 - Add tests for LayoutActor and GridActor.
https://reviewboard.mozilla.org/r/131320/#review134790
I think we need another couple of checks. But thanks for adding tests!
::: commit-message-d5b67:1
(Diff revision 1)
> +Bug 1317215 - Add units tests for LayoutActor and GridActor. r=pbro
nit: these are not unit tests. I think the closest thing we have to unit tests in m-c is xpcshell tests. So let's just rephase this to "Add tests for LayoutActor and GridActor"
::: devtools/server/tests/browser/browser_layout_getAllGrids.js:15
(Diff revision 1)
> + let {client, walker, layout} = yield initLayoutFrontForUrl(MAIN_DOMAIN + "grid.html");
> + let grids = yield layout.getAllGrids(walker.rootNode, true);
> + let grid = grids[0];
> +
> + is(grids.length, 1, "One grid was returned.");
> + ok(grid.gridFragments, "The grid has grid fragments.");
Also check that gridFragments is an array.
::: devtools/server/tests/browser/browser_layout_getAllGrids.js:16
(Diff revision 1)
> + let grids = yield layout.getAllGrids(walker.rootNode, true);
> + let grid = grids[0];
> +
> + is(grids.length, 1, "One grid was returned.");
> + ok(grid.gridFragments, "The grid has grid fragments.");
> +
This test (or maybe another one) should also check the contents of a gridFragment. Check that it contains the right properties, and their format.
Attachment #8859306 -
Flags: review?(pbrosset)
Comment hidden (mozreview-request) |
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8859306 [details]
Bug 1317215 - Add tests for LayoutActor and GridActor.
https://reviewboard.mozilla.org/r/131320/#review136702
::: devtools/server/tests/browser/browser_layout_getAllGrids.js:119
(Diff revisions 1 - 2)
> + let { gridFragments } = grid;
>
> is(grids.length, 1, "One grid was returned.");
> - ok(grid.gridFragments, "The grid has grid fragments.");
> + is(gridFragments.length, 1, "One grid fragment was returned.");
> + ok(Array.isArray(gridFragments), "An array of grid fragments was returned.");
> + is(JSON.stringify(gridFragments[0]), JSON.stringify(GRID_FRAGMENT_DATA),
I think you can use `deepEqual` instead of `JSON.stringify` to compare these objects.
Attachment #8859306 -
Flags: review?(pbrosset) → review+
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3f163644c78
Add tests for the LayoutActor and GridActor. r=pbro
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/379d1f58868f
Part 2: Fix eslint errors with browser_layout_getAllGrids.js and devtools/server/tests/browser/head.js. r=me
Comment 8•8 years ago
|
||
Sorry had to back this out for devtool failures at browser_layout_getAllGrids.js like https://treeherder.mozilla.org/logviewer.html#?job_id=94711793&repo=mozilla-inbound&lineNumber=3447
Flags: needinfo?(gl)
Backout by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d530c8f787e2
Backed out changeset 379d1f58868f for devtool failures at browser_layout_getAllGrids.js. a=backout
https://hg.mozilla.org/integration/mozilla-inbound/rev/eec8ca3761b5
Backed out changeset b3f163644c78 . a=backout
Comment 10•8 years ago
|
||
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc8dfed5a5d2
Add tests for the LayoutActor and GridActor. r=pbro
Comment 11•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(gl)
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•