Closed Bug 1462055 Opened 2 years ago Closed 2 years ago

Fix webconsole mocha tests / require paths

Categories

(DevTools :: Console, enhancement)

enhancement
Not set
normal

Tracking

(firefox62 fixed)

RESOLVED FIXED
Firefox 62
Tracking Status
firefox62 --- fixed

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(1 file)

Looks like some require() changes, plus the introduction of the JSTerm component has caused the tests to fail.
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.
Flags: needinfo?(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
Attachment #8976252 - Flags: review+
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Alright, going to land this version and then we can look into the remaining failure and actually requiring from the loader methods later on.
Flags: needinfo?(nchevobbe)
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)
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8347dbb954a9
Fixup mocha test paths for webconsole;r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/8347dbb954a9
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.