Derived from bug 1764682

bug 1764682 is caused by findMessage testing function returns console input, while the code should wait for console API output.

There seems to be similar situation in other tests, for the case that the test executes code from console and wait for output:

https://searchfox.org/mozilla-central/rev/d34f9713ae128a3138c2b70d8041a535f1049d19/devtools/client/webconsole/test/browser/browser_console_chrome_context_message.js#21-30

  const expectedMessages = [
    `Cu.reportError`, // bug 1561930
  ];

...
  execute(hud, `Cu.reportError("Cu.reportError");`); // bug 1561930
...
  await waitFor(() =>
    expectedMessages.every(expectedMessage => findMessage(hud, expectedMessage))

Those tests should pass special selector to the 3rd parameter of findMessage.
(such as ".console-api" for console.log, not sure for Cu.reportError, maybe just :not(.command) ?)

Or maybe we should make the default behavior ignore the console input (".message:not(.command)")?

testing patch that ignores command by default
https://treeherder.mozilla.org/jobs?repo=try&group_state=expanded&revision=eed157b8cc37c6186a99e4d159a6d42e05ea8db2

there are some failure, that may be just that it's expecting command input, or perhaps some other issue.

https://hg.mozilla.org/mozilla-central/rev/150bf969aea0ec4434daccd86c52cc96f0f3c02a
The patch suggests that making the selector required parameter might be better.
or maybe provide separate function for ".message.command", ".message.result", etc

to avoid surprise by changing the function signature or parameter requirement,
maybe we could do this at the same time with bug 1669126, with single implementation across all tests, with possibly different function name.

there's already some functions in some shared-head.js, but some tests are still defining their own version.
https://searchfox.org/mozilla-central/search?q=function%20findMessage&path=

:nchevobbe, can I have your opinion here?

How do you think about making the selector parameter required?
Also, what do you think about adding new function (e.g. findMessageByClass) instead of modifying the existing function?

What I'm thinking right now is, introducing findMessageByClass(hud, "hello", ".console-api") and waitForMessageByClass(hud, "hello", ".error") etc.
The caller needs to pass the "message type"-specific class name, instead of the entire query, and internally add ".message" to generate the query.
(just to reduce the redundancy)

Those functions will be added to shared head.js file, and then replace each consumer with ByClass variants, fixing some unexpected cases (e.g. comment #0) at the same time, and once the replacement completes, remove findMessage functions where selector is optional,
so that new tests written from that point are required to specify class, and there's no new bug around finding/waiting for wrong message.

(In reply to Tooru Fujisawa [:arai] from comment #4)

:nchevobbe, can I have your opinion here?

How do you think about making the selector parameter required?
Also, what do you think about adding new function (e.g. findMessageByClass) instead of modifying the existing function?

What I'm thinking right now is, introducing findMessageByClass(hud, "hello", ".console-api") and waitForMessageByClass(hud, "hello", ".error") etc.
The caller needs to pass the "message type"-specific class name, instead of the entire query, and internally add ".message" to generate the query.
(just to reduce the redundancy)

Those functions will be added to shared head.js file, and then replace each consumer with ByClass variants, fixing some unexpected cases (e.g. comment #0) at the same time, and once the replacement completes, remove findMessage functions where selector is optional,
so that new tests written from that point are required to specify class, and there's no new bug around finding/waiting for wrong message.

I like your proposal, overall it makes sense to force passing the classname so we don't get false positive.
We might introduce type-specific finder then (e.g. findEvaluationResultMessage, findErrorMessage, …) if we feel like callsites are a bit harder to read.

Thank you!
I'll work on it.

Just realized ByClass is misleading that the parameter is selector, not class name.
I'll go with ByType

I have question regarding findMessage(s)Virtualized vs findMessage(s).

findMessage(s)Virtualized seems to be used in very limited place, and most place uses findMessage(s).
Is it intentional, or all consumers are supposed to switch to findMessage(s)Virtualized ?
I'm wondering whether the new ByType functions should be based on findMessage(s)Virtualized algorithm or findMessage(s) algorithm.

here's WIP based on findMessage(s), that doesn't scroll.
https://treeherder.mozilla.org/jobs?repo=try&revision=b5091293640b8fe5abf7fa1a0ec573581c6b1263

(In reply to Tooru Fujisawa [:arai] from comment #8)

I have question regarding findMessage(s)Virtualized vs findMessage(s).

findMessage(s)Virtualized seems to be used in very limited place, and most place uses findMessage(s).
Is it intentional, or all consumers are supposed to switch to findMessage(s)Virtualized ?
I'm wondering whether the new ByType functions should be based on findMessage(s)Virtualized algorithm or findMessage(s) algorithm.

When we added virtualization, we added the findMessageVirtualized helper to use it in tests that were failing.
Lots of test didn't have to switch to the new helpers as they don't have enough message for the virtualization to kick in

So IMO the ByType function should be based on findMessage(s) , but maybe we could also have findMessage(s)VirtualizedByType at the same time (or later) ?

Depends on D147021

Depends on D147022

Depends on D147023

Depends on D147024

Depends on D147025

Depends on D147026

Depends on D147027

Depends on D147028

Depends on D147029

Depends on D147030

Depends on D147031

Depends on D147032

Depends on D147033

Depends on D147034

Depends on D147035

Depends on D147036

Depends on D147037

Depends on D147038

Depends on D147039

Depends on D147040

Depends on D147041

Depends on D147042

Depends on D147043

Depends on D147044

Depends on D147045

Depends on D147046

https://hg.mozilla.org/mozilla-central/rev/04b84879d85e
https://hg.mozilla.org/mozilla-central/rev/f35df94ea1d5
https://hg.mozilla.org/mozilla-central/rev/2a5382583807
https://hg.mozilla.org/mozilla-central/rev/2abb293b94c8
https://hg.mozilla.org/mozilla-central/rev/df95e92908e0
https://hg.mozilla.org/mozilla-central/rev/b8075ad20646
https://hg.mozilla.org/mozilla-central/rev/189891fa3067
https://hg.mozilla.org/mozilla-central/rev/e96bc7b205dc
https://hg.mozilla.org/mozilla-central/rev/15d1924a3ecf
https://hg.mozilla.org/mozilla-central/rev/6378c2ef157a
https://hg.mozilla.org/mozilla-central/rev/1082127c4df2
https://hg.mozilla.org/mozilla-central/rev/1ee9606f67eb
https://hg.mozilla.org/mozilla-central/rev/e83c3e63ca63
https://hg.mozilla.org/mozilla-central/rev/0f2347d0303f
https://hg.mozilla.org/mozilla-central/rev/2975998b65ed
https://hg.mozilla.org/mozilla-central/rev/deff7858a2d5
https://hg.mozilla.org/mozilla-central/rev/099f223528fa
https://hg.mozilla.org/mozilla-central/rev/ca07dc8f01b2
https://hg.mozilla.org/mozilla-central/rev/861d0cd7b2fa
https://hg.mozilla.org/mozilla-central/rev/ef8b0bc412cb
https://hg.mozilla.org/mozilla-central/rev/94a81659571f
https://hg.mozilla.org/mozilla-central/rev/bc6db1de8b6f
https://hg.mozilla.org/mozilla-central/rev/aef55b5eacb1
https://hg.mozilla.org/mozilla-central/rev/385713053465
https://hg.mozilla.org/mozilla-central/rev/6c41fd93e6cb
https://hg.mozilla.org/mozilla-central/rev/8a5c5a21ebe1
https://hg.mozilla.org/mozilla-central/rev/abb6eeffb44d