Closed Bug 1317215 Opened 8 years ago Closed 8 years ago

Add unit tests for LayoutActor and GridActor

Categories

(DevTools :: Inspector, defect, P3)

defect

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.
Attached patch 1317215.patch [WIP] (obsolete) (deleted) — Splinter Review
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.
Priority: -- → P3
Blocks: 1347964
Assignee: nobody → gl
Status: NEW → ASSIGNED
Attachment #8810290 - Attachment is obsolete: true
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 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
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
Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fc8dfed5a5d2 Add tests for the LayoutActor and GridActor. r=pbro
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Flags: needinfo?(gl)
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: