Closed
Bug 1356474
Opened 8 years ago
Closed 7 years ago
Unit tests for the Grid Outline component
Categories
(DevTools :: Inspector, enhancement, P3)
DevTools
Inspector
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)
(deleted),
patch
|
gl
:
review+
|
Details | Diff | Splinter Review |
The grid outline component needs unit tests. These tests should especially verify correctness of extreme case usage of the grid outline.
Updated•8 years ago
|
Priority: -- → P3
Comment 1•7 years ago
|
||
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).
Updated•7 years ago
|
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → tigleym
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
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+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
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
Comment 8•7 years ago
|
||
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/0156ad061c5d for failing something most of the time, see for example https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=830842118f8a0f636aa298dd27a34918d924f746&filter-failure_classification_id=2
Comment 9•7 years ago
|
||
Attachment #8875186 -
Attachment is obsolete: true
Attachment #8878279 -
Flags: review+
Comment 10•7 years ago
|
||
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
Comment 11•7 years ago
|
||
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
Comment 12•7 years ago
|
||
Attachment #8878279 -
Attachment is obsolete: true
Attachment #8878307 -
Flags: review+
Comment 13•7 years ago
|
||
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
Comment 14•7 years ago
|
||
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)
Comment 15•7 years ago
|
||
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
Comment 16•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Assignee | ||
Comment 17•7 years ago
|
||
(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)
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•