1590630 - Duplicated modules tests in metrics test fail with --enable-debug-js-modules

Status: RESOLVED FIXED

(DevTools :: General, task, P3)

Description

[Julian Descottes [:jdescottes]]

STRs:

• add --enable-debug-js-modules in mozconfig
• run ./mach test devtools/client/framework/test/metrics/browser_metrics_inspector.js

ER: Test should pass
AR: Test fails with

devtools/client/framework/test/metrics/browser_metrics_inspector.js
  FAIL Whitelisted module not found in the duplicated modules: [resource://devtools/client/shared/vendor/react.js]. Whitelist should be updated. - 
Stack trace:
chrome://mochikit/content/browser-test.js:test_ok:1297
chrome://mochitests/content/browser/devtools/client/framework/test/metrics/head.js:runDuplicatedModulesTest:131
chrome://mochitests/content/browser/devtools/client/framework/test/metrics/browser_metrics_inspector.js:null:28
chrome://mochikit/content/browser-test.js:Tester_execTest/<:1067
chrome://mochikit/content/browser-test.js:Tester_execTest:1102
chrome://mochikit/content/browser-test.js:nextTest/<:930
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:805
  FAIL Whitelisted module not found in the duplicated modules: [resource://devtools/client/shared/vendor/react-prop-types.js]. Whitelist should be updated. - 
Stack trace:
chrome://mochikit/content/browser-test.js:test_ok:1297
chrome://mochitests/content/browser/devtools/client/framework/test/metrics/head.js:runDuplicatedModulesTest:131
chrome://mochitests/content/browser/devtools/client/framework/test/metrics/browser_metrics_inspector.js:null:28
chrome://mochikit/content/browser-test.js:Tester_execTest/<:1067
chrome://mochikit/content/browser-test.js:Tester_execTest:1102
chrome://mochikit/content/browser-test.js:nextTest/<:930
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:805

The test relies on a whitelist of duplicated modules, which includes react.js (very often loaded twice: once by the toolbox, once by the panel). But with --enable-js-module, we load react-dev.js so I guess this confuses the test.

We should:

• either make the test pass
• fail the test explicitly when --enable-debug-js-modules is on and tell the user they need to disable this pref before running the test

I think I prefer the second option here, because since it's a performance metric, it shouldn't really be run in "debug" configurations

Comment 1

[Julian Descottes [:jdescottes]]

Interestingly in --enable-debug-js-modules mode, we load both react-dev.js and react.js...

This is most likely because the toolbox loads react using a Browser Loader, but the inspector doesn't.

The usage of React in the inspector is quite inconsistent.
inspector.js defines a few helper to reuse the toolbox's instance of React:

  get React() {
    return this._toolbox.React;
  },

  get ReactDOM() {
    return this._toolbox.ReactDOM;
  },

  get ReactRedux() {
    return this._toolbox.ReactRedux;
  },

but then the sidepanels usually load new versions of React, using the regular DevTools loader, instead of a BrowserLoader. Since the mapping react -> react-dev is only applied on the Browser Loader, we get this behavior in the test.

Not new and not in scope of this bug, but this explains why the test fails like that.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1590630 - Fail DevTools metrics test explicitly when DEBUG_JS_MODULES is enabled

DEBUG_JS_MODULES will make the BrowserLoader load different versions of some React files.
This confuses our metrics test that checks against duplicated modules and relies on a strict whitelist.
We fail the test explicitly since this performance metrics test should really only be run without DEBUG modules.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3540d62f0679
Fail DevTools metrics test explicitly when DEBUG_JS_MODULES is enabled r=daisuke

Comment 4

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/3540d62f0679