1403450 - Migrate browser_repeated_messages_accuracy.js to mocha test

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Description

[Nicolas Chevobbe [:nchevobbe]]

This test has a better coverage of the repeat feature that what we have in our mocha tests. 
The cases we don't have should be added in the store test where we test repeat.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1403450 - Add test cases from browser_webconsole_repeated_messages_accuracy.js to mocha; .

This test had some cases that weren't covered in our mocha tests,
so we add them. In order to do that, we added a stub.
We remove the mochhitest and its support file since it's not
needed anymore.

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

Comment 2

[Julian Descottes [:jdescottes]]

Comment on attachment 8950498 [details]
Bug 1403450 - Add test cases from browser_webconsole_repeated_messages_accuracy.js to mocha; .

https://reviewboard.mozilla.org/r/219784/#review225534

While running the tests I got a bunch of "Cannot read property '_renderedComponent' of undefined", failing 17 tests in the mocha test suite. I seem to remember seeing something about that on slack so I guess it's unrelated to this patch.

Clearing the r? to see what we are planning for browser_webconsole_repeat_different_objects.js.

::: devtools/client/webconsole/new-console-output/test/mochitest/browser.ini
(Diff revision 1)
>    test-network.html
>    test-observe-http-ajax.html
>    test-own-console.html
>    test-property-provider.html
>    test-reopen-closed-tab.html
> -  test-repeated-messages.html

This support file is still used in browser_webconsole_repeat_different_objects.js.

I don't think messages.test.js currently covers the "non repeated object" being asserted by this test.

We can either:
- migrate it to mocha
- remove support file, use dummy data-uri and replace 
    hud.jsterm.execute("window.testConsoleObjects()");
  by
    hud.jsterm.execute(
      `for (var i = 0; i < 3; i++) {
        console.log("abba", { id: "abba" });
      }`
    );

::: devtools/client/webconsole/new-console-output/test/store/messages.test.js:73
(Diff revision 1)
> +      const key1 = "console.log('foobar', 'test')";
> +      const { dispatch, getState } = setupStore();
> +
> +      const packet = clonePacket(stubPackets.get(key1));
> +
> +      // Repeat ID must be the same even if the timestamp is different.

Wrong comment?

// Dispatch original packet
...
// Dispatch same packet with modified column number
...
// Dispatch same packed with modified line number

::: devtools/client/webconsole/new-console-output/test/store/messages.test.js:114
(Diff revision 1)
> +      const key1 = "Unknown property ‘such-unknown-property’.  Declaration dropped.";
> +      const { dispatch, getState } = setupStore();
> +
> +      const packet = clonePacket(stubPackets.get(key1));
> +
> +      // Repeat ID must be the same even if the timestamp is different.

ditto: Wrong comment

::: devtools/client/webconsole/new-console-output/test/store/messages.test.js:184
(Diff revision 1)
> +
> +      const repeat = getAllRepeatById(getState());
> +      expect(Object.keys(repeat).length).toBe(0);
> +    });
> +
> +    it("doesn't increment successive falsy but unsimilar messages", () => {

nit: unsimilar -> different? (unsimilar is valid but rarely used in this way)

::: devtools/client/webconsole/new-console-output/test/store/messages.test.js:196
(Diff revision 1)
> +      let messages = getAllMessagesById(getState());
> +      expect(messages.size).toBe(3);
> +      let repeat = getAllRepeatById(getState());
> +      expect(Object.keys(repeat).length).toBe(0);
> +
> +      const nanPacket = stubPackets.get("console.log(NaN)");

Starting here we are testing that identical falsy messages are indeed repeated. 

Either move to its own test, or update the test description to it("increments falsy messages when expected") (plus add comments in the test to highlight the different things being tested).

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8950498 [details]
Bug 1403450 - Add test cases from browser_webconsole_repeated_messages_accuracy.js to mocha; .

https://reviewboard.mozilla.org/r/219784/#review225534

yes, the `_renderedComponent` issue is due to the update to React 16, and will be fixed by Bug 1436690

> This support file is still used in browser_webconsole_repeat_different_objects.js.
> 
> I don't think messages.test.js currently covers the "non repeated object" being asserted by this test.
> 
> We can either:
> - migrate it to mocha
> - remove support file, use dummy data-uri and replace 
>     hud.jsterm.execute("window.testConsoleObjects()");
>   by
>     hud.jsterm.execute(
>       `for (var i = 0; i < 3; i++) {
>         console.log("abba", { id: "abba" });
>       }`
>     );

good catch. For now I'll switch to a dataURI

> Wrong comment?
> 
> // Dispatch original packet
> ...
> // Dispatch same packet with modified column number
> ...
> // Dispatch same packed with modified line number

yes

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8950498 [details]
Bug 1403450 - Add test cases from browser_webconsole_repeated_messages_accuracy.js to mocha; .

https://reviewboard.mozilla.org/r/219784/#review225658

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8950498 [details]
Bug 1403450 - Add test cases from browser_webconsole_repeated_messages_accuracy.js to mocha; .

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

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8950498 [details]
Bug 1403450 - Add test cases from browser_webconsole_repeated_messages_accuracy.js to mocha; .

https://reviewboard.mozilla.org/r/219784/#review225662

Great, thanks for addressing the comments Nicolas!

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8950498 [details]
Bug 1403450 - Add test cases from browser_webconsole_repeated_messages_accuracy.js to mocha; .

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

Comment 8

[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 52e6deffbaf0277f941b23f02bdff2660299bae1 -d 04ddbc05db87: rebasing 447230:52e6deffbaf0 "Bug 1403450 - Add test cases from browser_webconsole_repeated_messages_accuracy.js to mocha; r=jdescottes." (tip)
merging devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/stub-snippets.js
merging devtools/client/webconsole/new-console-output/test/fixtures/stubs/evaluationResult.js
merging devtools/client/webconsole/new-console-output/test/mochitest/browser.ini
warning: conflicts while merging devtools/client/webconsole/new-console-output/test/fixtures/stubs/evaluationResult.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8950498 [details]
Bug 1403450 - Add test cases from browser_webconsole_repeated_messages_accuracy.js to mocha; .

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b6f87dae8ad
Add test cases from browser_webconsole_repeated_messages_accuracy.js to mocha; r=jdescottes.

Comment 11

[Mozilla Autoland]

We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/0b6f87dae8ad