Comment on attachment 9024558 [details] [diff] [review] 1476395.patch [1.0] Review of attachment 9024558 [details] [diff] [review]: ----------------------------------------------------------------- One comment about the commit message: these aren't unit tests, these are either integration tests, or maybe just mochitests if we want to be more specific. But they're not unit tests for sure. Could you please update the commit message? Also I have 2 minor comments below. But R+ with a green try build and those things addressed. Thanks! ::: devtools/client/inspector/flexbox/test/browser_flexbox_empty_state.js @@ +6,5 @@ > +// Test that a message is displayed when no flex container is selected. > + > +const TEST_URI = ` > + <style type='text/css'> > + </style> No need for an empty <style> tag in here. Simply having the <div></div> markup would be enough. ::: devtools/client/inspector/flexbox/test/browser_flexbox_toggle_flexbox_highlighter_02.js @@ +27,5 @@ > + let onHighlighterShown = highlighters.once("flexbox-highlighter-shown"); > + let onToggleChange = waitUntilState(store, state => state.flexbox.highlighted); > + flexHighlighterToggle.click(); > + await onHighlighterShown; > + await onToggleChange; We repeat (roughly) these 6 lines of code several times in this test and in the previous test too. Could you please move this logic to a new function in the nearby heard.js file? Something like toggleHighlighter(state)

Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/86005ec774e6 Add mochitests for the flexbox inspector sidebar. r=pbro