Closed Bug 1356474 Opened 8 years ago Closed 7 years ago

Unit tests for the Grid Outline component

Categories

(DevTools :: Inspector, enhancement, P3)

enhancement

Tracking

(firefox56 fixed)

RESOLVED FIXED
Firefox 56
Tracking Status
firefox56 --- fixed

People

(Reporter: micah, Assigned: micah)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

The grid outline component needs unit tests. These tests should especially verify correctness of extreme case usage of the grid outline.
Priority: -- → P3
We should take in account the changes made by bug 1348919 here: since we do not have specific test for the `moveInfobar` function, but we're testing the highlighters that are using it. In the grid outline test, we should add some tests about the grid highlighters' infobars too (e.g. checking that they're hidden when offscreen, and they're properly positioned on top / bottom).
Assignee: nobody → tigleym
Status: NEW → ASSIGNED
Comment on attachment 8875186 [details] Bug 1356474 - Add units tests for the Grid Outline component. https://reviewboard.mozilla.org/r/146590/#review151348 ::: commit-message-1e315:1 (Diff revision 2) > +Bug 1356474 - Unit tests for Grid Outline. r?gl Add unit tests for the Grid Outline component. ::: devtools/client/inspector/grids/components/GridOutline.js:345 (Diff revision 2) > } = this.state; > > return showOutline ? > dom.svg( > { > + id: "grid-outline-container", Change this to "grid-outline" ::: devtools/client/inspector/grids/components/GridOutline.js:362 (Diff revision 2) > const { selectedGrid } = this.state; > > return selectedGrid ? > dom.div( > { > + id: "grid-outline", I think we should change "grid-outline" to "grid-outline-container" here, and rename the CSS as necessary. ::: devtools/client/inspector/grids/test/browser.ini:28 (Diff revision 2) > [browser_grids_grid-list-on-mutation-element-added.js] > [browser_grids_grid-list-on-mutation-element-removed.js] > [browser_grids_grid-list-toggle-multiple-grids.js] > [browser_grids_grid-list-toggle-single-grid.js] > [browser_grids_highlighter-setting-rules-grid-toggle.js] > +[browser_grids_grid-outline-selected-grid.js] This list needs to be alphabetically ordered. It should follow the order that you would see when you look at the directory list. ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-cannot-show-outline.js:6 (Diff revision 2) > +/* Any copyright is dedicated to the Public Domain. > + http://creativecommons.org/publicdomain/zero/1.0/ */ > + > +"use strict"; > + > +// Tests that outline does not show when cells are too small to be drawn and that "Cannot s/outline/grid outline ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-cannot-show-outline.js:46 (Diff revision 2) > + state.grids[0].highlighted); > + checkbox.click(); > + yield onHighlighterShown; > + yield onCheckboxChange; > + > + outline = doc.getElementById("grid-outline-container"); With the changes mentioned in the grid-outline naming changes, I think we can remove this since we are reassigning the outline anyways. ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-cannot-show-outline.js:49 (Diff revision 2) > + yield onCheckboxChange; > + > + outline = doc.getElementById("grid-outline-container"); > + let cannotShowGridOutline = doc.querySelector(".grid-outline-text"); > + > + info("Checking the grid outline is not rendered."); Checking the grid outline is not rendered and an appropriate message is shown. ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-cannot-show-outline.js:52 (Diff revision 2) > + let cannotShowGridOutline = doc.querySelector(".grid-outline-text"); > + > + info("Checking the grid outline is not rendered."); > + ok(!outline, "Outline component is shown."); > + > + info("Checking the message 'Cannot shown outline for this grid.' is shown."); Remove this info since we are rewording the previous info. ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-highlight-areas.js:6 (Diff revision 2) > +/* Any copyright is dedicated to the Public Domain. > + http://creativecommons.org/publicdomain/zero/1.0/ */ > + > +"use strict"; > + > +// Tests that outline changes when grid areas are highlighted on mouseover. Tests that the grid area and cell are highlighted when mouseovering a grid area in the grid outline. ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-highlight-areas.js:50 (Diff revision 2) > + state.grids[0].highlighted); > + checkbox.click(); > + yield onHighlighterShown; > + yield onCheckboxChange; > + > + let gridOutline = doc.querySelector("#grid-outline-container"); #grid-outline ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-highlight-areas.js:52 (Diff revision 2) > + yield onHighlighterShown; > + yield onCheckboxChange; > + > + let gridOutline = doc.querySelector("#grid-outline-container"); > + let gridCellA = gridOutline.children[0].querySelectorAll( > + "[data-grid-id = '0'][data-grid-row = '1'][data-grid-column='1']"); s/ = /= ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-highlight-areas.js:54 (Diff revision 2) > + > + let gridOutline = doc.querySelector("#grid-outline-container"); > + let gridCellA = gridOutline.children[0].querySelectorAll( > + "[data-grid-id = '0'][data-grid-row = '1'][data-grid-column='1']"); > + > + info("Scrolling into the view of the #grid-outline-container."); #grid-outline ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-highlight-areas.js:60 (Diff revision 2) > + gridCellA[0].scrollIntoView(); > + > + info("Hovering over grid cell A in the grid outline."); > + let onCellAHighlight = highlighters.once("grid-highlighter-shown", > + (event, nodeFront, options) => { > + info("Checking show grid cell / areas options are correct."); Checking the grid highlighter options for the show grid area and cell parameters. ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-highlight-areas.js:67 (Diff revision 2) > + const { gridFragmentIndex, rowNumber, columnNumber } = showGridCell; > + > + ok(showGridCell, "Show grid cell options are available."); > + ok(showGridArea, "Show grid areas options are available."); > + > + info("Checking params passed to showGridArea options are correct."); Remove this info with the rewording of the prev. info ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-highlight-areas.js:72 (Diff revision 2) > + info("Checking params passed to showGridArea options are correct."); > + is(gridFragmentIndex, 0, "Should be the first grid fragment index."); > + is(rowNumber, 1, "Should be the first grid row."); > + is(columnNumber, 1, "Should be the first grid column."); > + > + info("Checking grid area name is correct."); Remove this info. We only want info() for actions and checks. We usually already have another info messages with the is() and ok() checks to know what exactly we are checking for. For info() involving checks we just want to say what we are checking in general. ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-highlight-cell.js:6 (Diff revision 2) > +/* Any copyright is dedicated to the Public Domain. > + http://creativecommons.org/publicdomain/zero/1.0/ */ > + > +"use strict"; > + > +// Tests that outline changes when grid cell is highlighted on mouseover. Tests that the grid cell is highlighted when mouseovering the grid outline of a grid cell. ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-highlight-cell.js:42 (Diff revision 2) > + yield onHighlighterShown; > + yield onCheckboxChange; > + > + let gridOutline = doc.querySelector("#grid-outline-container"); > + let gridCellA = gridOutline.children[0].querySelectorAll( > + "[data-grid-id = '0'][data-grid-row = '1'][data-grid-column='1']"); s/ = /= ::: devtools/client/inspector/grids/test/browser_grids_grid-outline-selected-grid.js:6 (Diff revision 2) > +/* Any copyright is dedicated to the Public Domain. > + http://creativecommons.org/publicdomain/zero/1.0/ */ > + > +"use strict"; > + > +// Tests that outline is shown when a grid container is selected. Tests that the grid outline is shown when a grid container is selected ::: devtools/client/inspector/layout/layout.js:18 (Diff revision 2) > > const { LocalizationHelper } = require("devtools/shared/l10n"); > const INSPECTOR_L10N = > new LocalizationHelper("devtools/client/locales/inspector.properties"); > > const SHOW_GRID_OUTLINE_PREF = "devtools.gridinspector.showGridOutline"; This needs to be removed as well.
Attachment #8875186 - Flags: review?(gl) → review+
Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/830842118f8a Add units tests for the Grid Outline component. r=gl
Attached patch 1356474.patch (obsolete) (deleted) — Splinter Review
Attachment #8875186 - Attachment is obsolete: true
Attachment #8878279 - Flags: review+
Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ac1520300fc7 Add units tests for the Grid Outline component. r=gl
More consistent failure mode this time, anyway. Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/b4f50a8eb778 for "browser_grids_grid-outline-cannot-show-outline.js | The message 'Cannot show outline for this grid' is displayed." like https://treeherder.mozilla.org/logviewer.html#?job_id=107523714&repo=mozilla-inbound and https://treeherder.mozilla.org/logviewer.html#?job_id=107526665&repo=mozilla-inbound and https://treeherder.mozilla.org/logviewer.html#?job_id=107522810&repo=mozilla-inbound, and just one outlier browser_grids_grid-outline-selected-grid.js failure, https://treeherder.mozilla.org/logviewer.html#?job_id=107530497&repo=mozilla-inbound
Attached patch 1356474.patch (deleted) — Splinter Review
Attachment #8878279 - Attachment is obsolete: true
Attachment #8878307 - Flags: review+
Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/955e237fc290 Add units tests for the Grid Outline component. r=gl
Sorry, this had to be backed out for frequently failing own test browser_grids_grid-outline-selected-grid.js: https://hg.mozilla.org/integration/mozilla-inbound/rev/4b1ea65170bbf4031fc6ee44a29d47a84afe77d1 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=955e237fc290e79eecface60d9b1af4d2abe293b&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=107583079&repo=mozilla-inbound [task 2017-06-16T07:08:07.971562Z] 07:08:07 INFO - Entering test bound [task 2017-06-16T07:08:07.972174Z] 07:08:07 INFO - Adding a new tab with URL: data:text/html;charset=utf-8,%0A%20%20%3Cstyle%20type%3D'text%2Fcss'%3E%0A%20%20%20%20%23grid%20%7B%0A%20%20%20%20%20%20display%3A%20grid%3B%0A%20%20%20%20%7D%0A%20%20%3C%2Fstyle%3E%0A%20%20%3Cdiv%20id%3D%22grid%22%3E%0A%20%20%20%20%3Cdiv%20id%3D%22cella%22%3ECell%20A%3C%2Fdiv%3E%0A%20%20%20%20%3Cdiv%20id%3D%22cellb%22%3ECell%20B%3C%2Fdiv%3E%0A%20%20%20%20%3Cdiv%20id%3D%22cellc%22%3ECell%20C%3C%2Fdiv%3E%0A%20%20%3C%2Fdiv%3E%0A [task 2017-06-16T07:08:07.972311Z] 07:08:07 INFO - Tab added and finished loading [task 2017-06-16T07:08:07.972394Z] 07:08:07 INFO - Opening the inspector [task 2017-06-16T07:08:07.973910Z] 07:08:07 INFO - Opening the toolbox [task 2017-06-16T07:08:07.976434Z] 07:08:07 INFO - Buffered messages logged at 07:07:23 [task 2017-06-16T07:08:07.978496Z] 07:08:07 INFO - Toolbox opened and focused [task 2017-06-16T07:08:07.980583Z] 07:08:07 INFO - Waiting for actor features to be detected [task 2017-06-16T07:08:07.982404Z] 07:08:07 INFO - Selecting the layoutview sidebar [task 2017-06-16T07:08:07.983910Z] 07:08:07 INFO - Checking the initial state of the Grid Inspector. [task 2017-06-16T07:08:07.985481Z] 07:08:07 INFO - TEST-PASS | devtools/client/inspector/grids/test/browser_grids_grid-outline-selected-grid.js | There should be no grid outline shown. - [task 2017-06-16T07:08:07.987756Z] 07:08:07 INFO - Toggling ON the CSS grid highlighter from the layout panel. [task 2017-06-16T07:08:07.989888Z] 07:08:07 INFO - Waiting for state predicate "state => [task 2017-06-16T07:08:07.991198Z] 07:08:07 INFO - state.grids.length == 1 && [task 2017-06-16T07:08:07.992642Z] 07:08:07 INFO - state.grids[0].highlighted" [task 2017-06-16T07:08:07.994364Z] 07:08:07 INFO - Found state predicate "state => [task 2017-06-16T07:08:07.995367Z] 07:08:07 INFO - state.grids.length == 1 && [task 2017-06-16T07:08:07.996815Z] 07:08:07 INFO - state.grids[0].highlighted" [task 2017-06-16T07:08:07.998313Z] 07:08:07 INFO - Buffered messages logged at 07:07:24 [task 2017-06-16T07:08:07.999991Z] 07:08:07 INFO - Checking the grid outline is shown. [task 2017-06-16T07:08:08.001884Z] 07:08:08 INFO - TEST-PASS | devtools/client/inspector/grids/test/browser_grids_grid-outline-selected-grid.js | Grid outline is shown. - [task 2017-06-16T07:08:08.003658Z] 07:08:08 INFO - Toggling OFF the CSS grid highlighter from the layout panel. [task 2017-06-16T07:08:08.005586Z] 07:08:08 INFO - Waiting for state predicate "state => [task 2017-06-16T07:08:08.006920Z] 07:08:08 INFO - state.grids.length == 1 && [task 2017-06-16T07:08:08.008095Z] 07:08:08 INFO - !state.grids[0].highlighted" [task 2017-06-16T07:08:08.009260Z] 07:08:08 INFO - Found state predicate "state => [task 2017-06-16T07:08:08.010065Z] 07:08:08 INFO - state.grids.length == 1 && [task 2017-06-16T07:08:08.010779Z] 07:08:08 INFO - !state.grids[0].highlighted" [task 2017-06-16T07:08:08.011845Z] 07:08:08 INFO - Buffered messages finished [task 2017-06-16T07:08:08.013075Z] 07:08:08 INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/grids/test/browser_grids_grid-outline-selected-grid.js | Test timed out - Please fix the issue and submit an updated patch.
Flags: needinfo?(tigleym)
Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e9683155991f Add units tests for the Grid Outline component. r=gl
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #14) > Sorry, this had to be backed out for frequently failing own test > browser_grids_grid-outline-selected-grid.js: > > https://hg.mozilla.org/integration/mozilla-inbound/rev/ > 4b1ea65170bbf4031fc6ee44a29d47a84afe77d1 > > Push with failures: > https://treeherder.mozilla.org/#/jobs?repo=mozilla- > inbound&revision=955e237fc290e79eecface60d9b1af4d2abe293b&filter- > resultStatus=testfailed&filter-resultStatus=busted&filter- > resultStatus=exception&filter-resultStatus=retry&filter- > resultStatus=usercancel&filter-resultStatus=runnable > Failure log: > https://treeherder.mozilla.org/logviewer.html#?job_id=107583079&repo=mozilla- > inbound > > [task 2017-06-16T07:08:07.971562Z] 07:08:07 INFO - Entering test bound > [task 2017-06-16T07:08:07.972174Z] 07:08:07 INFO - Adding a new tab with > URL: > data:text/html;charset=utf-8, > %0A%20%20%3Cstyle%20type%3D'text%2Fcss'%3E%0A%20%20%20%20%23grid%20%7B%0A%20% > 20%20%20%20%20display%3A%20grid%3B%0A%20%20%20%20%7D%0A%20%20%3C%2Fstyle%3E%0 > A%20%20%3Cdiv%20id%3D%22grid%22%3E%0A%20%20%20%20%3Cdiv%20id%3D%22cella%22%3E > Cell%20A%3C%2Fdiv%3E%0A%20%20%20%20%3Cdiv%20id%3D%22cellb%22%3ECell%20B%3C%2F > div%3E%0A%20%20%20%20%3Cdiv%20id%3D%22cellc%22%3ECell%20C%3C%2Fdiv%3E%0A%20%2 > 0%3C%2Fdiv%3E%0A > [task 2017-06-16T07:08:07.972311Z] 07:08:07 INFO - Tab added and > finished loading > [task 2017-06-16T07:08:07.972394Z] 07:08:07 INFO - Opening the inspector > [task 2017-06-16T07:08:07.973910Z] 07:08:07 INFO - Opening the toolbox > [task 2017-06-16T07:08:07.976434Z] 07:08:07 INFO - Buffered messages > logged at 07:07:23 > [task 2017-06-16T07:08:07.978496Z] 07:08:07 INFO - Toolbox opened and > focused > [task 2017-06-16T07:08:07.980583Z] 07:08:07 INFO - Waiting for actor > features to be detected > [task 2017-06-16T07:08:07.982404Z] 07:08:07 INFO - Selecting the > layoutview sidebar > [task 2017-06-16T07:08:07.983910Z] 07:08:07 INFO - Checking the initial > state of the Grid Inspector. > [task 2017-06-16T07:08:07.985481Z] 07:08:07 INFO - TEST-PASS | > devtools/client/inspector/grids/test/browser_grids_grid-outline-selected- > grid.js | There should be no grid outline shown. - > [task 2017-06-16T07:08:07.987756Z] 07:08:07 INFO - Toggling ON the CSS > grid highlighter from the layout panel. > [task 2017-06-16T07:08:07.989888Z] 07:08:07 INFO - Waiting for state > predicate "state => > [task 2017-06-16T07:08:07.991198Z] 07:08:07 INFO - > state.grids.length == 1 && > [task 2017-06-16T07:08:07.992642Z] 07:08:07 INFO - > state.grids[0].highlighted" > [task 2017-06-16T07:08:07.994364Z] 07:08:07 INFO - Found state predicate > "state => > [task 2017-06-16T07:08:07.995367Z] 07:08:07 INFO - > state.grids.length == 1 && > [task 2017-06-16T07:08:07.996815Z] 07:08:07 INFO - > state.grids[0].highlighted" > [task 2017-06-16T07:08:07.998313Z] 07:08:07 INFO - Buffered messages > logged at 07:07:24 > [task 2017-06-16T07:08:07.999991Z] 07:08:07 INFO - Checking the grid > outline is shown. > [task 2017-06-16T07:08:08.001884Z] 07:08:08 INFO - TEST-PASS | > devtools/client/inspector/grids/test/browser_grids_grid-outline-selected- > grid.js | Grid outline is shown. - > [task 2017-06-16T07:08:08.003658Z] 07:08:08 INFO - Toggling OFF the CSS > grid highlighter from the layout panel. > [task 2017-06-16T07:08:08.005586Z] 07:08:08 INFO - Waiting for state > predicate "state => > [task 2017-06-16T07:08:08.006920Z] 07:08:08 INFO - > state.grids.length == 1 && > [task 2017-06-16T07:08:08.008095Z] 07:08:08 INFO - > !state.grids[0].highlighted" > [task 2017-06-16T07:08:08.009260Z] 07:08:08 INFO - Found state predicate > "state => > [task 2017-06-16T07:08:08.010065Z] 07:08:08 INFO - > state.grids.length == 1 && > [task 2017-06-16T07:08:08.010779Z] 07:08:08 INFO - > !state.grids[0].highlighted" > [task 2017-06-16T07:08:08.011845Z] 07:08:08 INFO - Buffered messages > finished > [task 2017-06-16T07:08:08.013075Z] 07:08:08 INFO - TEST-UNEXPECTED-FAIL > | > devtools/client/inspector/grids/test/browser_grids_grid-outline-selected- > grid.js | Test timed out - > > Please fix the issue and submit an updated patch. Has been fixed and resolved in gl 's latest patch.
Flags: needinfo?(tigleym)
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: