1382606 - Fix 45 tests failures on devtools/client/webconsole due the EventEmitter refactoring

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P2)

Description

[Matteo Ferretti [:zer0] [:matteo]]

Failing tests:

devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_context_menu_copy_link_location.js
devtools/client/webconsole/test/browser_bug664688_sandbox_update_after_navigation.js
devtools/client/webconsole/test/browser_bug_638949_copy_link_location.js
devtools/client/webconsole/test/browser_bug_862916_console_dir_and_filter_off.js
devtools/client/webconsole/test/browser_bug_865288_repeat_different_objects.js
devtools/client/webconsole/test/browser_bug_865871_variables_view_close_on_esc_key.js
devtools/client/webconsole/test/browser_bug_869003_inspect_cross_domain_object.js
devtools/client/webconsole/test/browser_cached_messages.js
devtools/client/webconsole/test/browser_console.js
devtools/client/webconsole/test/browser_console_addonsdk_loader_exception.js
devtools/client/webconsole/test/browser_console_clear_method.js
devtools/client/webconsole/test/browser_console_clear_on_reload.js
devtools/client/webconsole/test/browser_console_click_focus.js
devtools/client/webconsole/test/browser_console_consolejsm_output.js
devtools/client/webconsole/test/browser_console_copy_entire_message_context_menu.js
devtools/client/webconsole/test/browser_console_error_source_click.js
devtools/client/webconsole/test/browser_console_hide_jsterm_when_devtools_chrome_enabled_false.js
devtools/client/webconsole/test/browser_console_iframe_messages.js
devtools/client/webconsole/test/browser_console_keyboard_accessibility.js
devtools/client/webconsole/test/browser_console_log_inspectable_object.js
devtools/client/webconsole/test/browser_console_native_getters.js
devtools/client/webconsole/test/browser_console_navigation_marker.js
devtools/client/webconsole/test/browser_console_netlogging.js
devtools/client/webconsole/test/browser_console_nsiconsolemessage.js
devtools/client/webconsole/test/browser_console_open_or_focus.js
devtools/client/webconsole/test/browser_console_optimized_out_vars.js
devtools/client/webconsole/test/browser_console_private_browsing.js
devtools/client/webconsole/test/browser_console_server_logging.js
devtools/client/webconsole/test/browser_console_variables_view.js
devtools/client/webconsole/test/browser_console_variables_view_dom_nodes.js
devtools/client/webconsole/test/browser_console_variables_view_dont_sort_non_sortable_classes_properties.js
devtools/client/webconsole/test/browser_console_variables_view_filter.js
devtools/client/webconsole/test/browser_console_variables_view_highlighter.js
devtools/client/webconsole/test/browser_console_variables_view_special_names.js
devtools/client/webconsole/test/browser_console_variables_view_while_debugging.js
devtools/client/webconsole/test/browser_console_variables_view_while_debugging_and_inspecting.js
devtools/client/webconsole/test/browser_longstring_hang.js
devtools/client/webconsole/test/browser_netmonitor_shows_reqs_in_webconsole.js
devtools/client/webconsole/test/browser_output_breaks_after_console_dir_uninspectable.js
devtools/client/webconsole/test/browser_output_longstring_expand.js
devtools/client/webconsole/test/browser_repeated_messages_accuracy.js
devtools/client/webconsole/test/browser_warn_user_about_replaced_api.js
devtools/client/webconsole/test/browser_webconsole_bug_587617_output_copy.js
devtools/client/webconsole/test/browser_webconsole_bug_613280_jsterm_copy.js
devtools/client/webconsole/test/browser_webconsole_output_copy_newlines.js


The refactoring is currently only on:

https://github.com/zer0/gecko/tree/event-emitter-1381542

We need to address the test failures before land this patch in m-c.

Comment 1

[Matteo Ferretti [:zer0] [:matteo]]

Here the original try build with the failures:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bba13e27a2371fa8aad68b9b227534b31829cb0d

Those failures are most likely due the breaking change in how the `EventEmitter` emits event.

Previously, the first argument was the type event:

  myEmitter.on("custom-event", (eventType, message) => { ... });

Now the first argument is the message:

  myEmitter.on("custom-event", (message) => { ... });

In the majority of the scenario the `eventType` is ignored by our code, so we should just remove it from the function's signature.

For more details, see: https://github.com/devtools-html/snippets-for-removing-the-sdk/#events

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Is this still needed ? I thought the no-sdk project was over, and I don't think we landed something that fail so many tests :)

Comment 3

[Patrick Brosset <:pbro>]

Yes, this bug is perhaps badly named, this isn't about the no-sdk refactoring, but instead about migrating to the new event-emitter.

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

I'll try to use the new EventEmitter in the new console frontend and see if tests are failing.
If not, we will be able to block this on the removal of the old frontend code.

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1382606 - Switch webconsole to new event-emitter; .

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

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8953516 [details]
Bug 1382606 - Switch webconsole to new event-emitter; .

https://reviewboard.mozilla.org/r/222754/#review229830

Sorry for the delayt, looks good to me.

R+, assuming try is green.

Honza

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d249a26e23a
Switch webconsole to new event-emitter; r=Honza.

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/8d249a26e23a