1462055 - Fix webconsole mocha tests / require paths

Status: RESOLVED FIXED

(DevTools :: Console, enhancement)

Description

[Brian Grinstead [:bgrins]]

Looks like some require() changes, plus the introduction of the JSTerm component has caused the tests to fail.

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: Bug 1462055 - Fixup mocha test paths for webconsole;

Review commit: https://reviewboard.mozilla.org/r/244442/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/244442/

Comment 2

[Brian Grinstead [:bgrins]]

This patch gets it running again and fixes all but one test:

  1) Network message reducer:
       networkMessagesUpdateById
         adds fetched HTTP post data:
     Error: Expected undefined to exist
      at assert (node_modules/expect/lib/assert.js:29:9)
      at Expectation.toExist (node_modules/expect/lib/Expectation.js:52:28)
      at Context.it (store/network-messages.test.js:82:70)

I'd be generally OK to get it landed and figure out the failing test later, but maybe there's something obvious here we can do to fix it.

Also, there's a change in JSTerm.js that I don't like that involved checking `if (!this.loader)` and defining it if it doesn't exist. Where can we define globals for mocha tests that aren't `require`d?  The webpack.config.js in client/webconsole seems to only be used for local dev.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8976252 [details]
Bug 1462055 - Fixup mocha test paths for webconsole;

https://reviewboard.mozilla.org/r/244442/#review250602

Looks good if we revert the loader change in JsTerm.js to put it in the mocha-test-setup

::: devtools/client/webconsole/components/JSTerm.js:10
(Diff revision 1)
> +if (!this.loader){
> +  var loader = {
> +    lazyServiceGetter: () => {},
> +    lazyRequireGetter: () => {}
> +  }
> +}

this can be removed by adding the following in the mocha-test-setup.js file: 

```js
global.loader = {
  lazyServiceGetter: () => {},
  lazyRequireGetter: () => {}
};
```

we might want to have a dedicated function/file (e.g. mockGlobals) to do this.

Also, maybe we should actually do the require when calling lazyRequireGetter ? For now it seems to work fine, so maybe this can go in a follow up

Comment 4

[Brian Grinstead [:bgrins]]

Comment on attachment 8976252 [details]
Bug 1462055 - Fixup mocha test paths for webconsole;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/244442/diff/1-2/

Comment 5

[Brian Grinstead [:bgrins]]

Comment on attachment 8976252 [details]
Bug 1462055 - Fixup mocha test paths for webconsole;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/244442/diff/2-3/

Comment 6

[Brian Grinstead [:bgrins]]

Alright, going to land this version and then we can look into the remaining failure and actually requiring from the loader methods later on.

Comment 7

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 94de9abbf353dbd1b0cc7fbc15b6b0c183e61ab3 -d fdd6d3850e7e: rebasing 464030:94de9abbf353 "Bug 1462055 - Fixup mocha test paths for webconsole;r=nchevobbe" (tip)
merging devtools/client/webconsole/components/JSTerm.js
warning: conflicts while merging devtools/client/webconsole/components/JSTerm.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 8976252 [details]
Bug 1462055 - Fixup mocha test paths for webconsole;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/244442/diff/3-4/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8347dbb954a9
Fixup mocha test paths for webconsole;r=nchevobbe

Comment 10

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/8347dbb954a9