1580155 - Rename event constants used only for tests

Status: RESOLVED FIXED

(DevTools :: Netmonitor, task, P3)

Description

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

This is a follow up for bug 1578215

See also this comment:
https://phabricator.services.mozilla.com/D44437#1367914

---

We should update the netmonitor constants.js file in order to clearly indicate which events are "test-only" and which events are "test and production":
https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/constants.js#85

We should have two constants "EVENTS" and "TEST_EVENTS".

Honza

Comment 1

[Jasmany Rocha]

Hello Mr Odvarko,

This is my first time tackling a bug for open source software. I would like to try and resolve this bug if that is okay with you.

Thank you,

Mr Jazz

Comment 2

[Shrey Gupta]

Hello Sir,
This is my first bug for Mozilla. I would like to try and contribute to this bug
Thank You

Comment 3

[Shrey Gupta]

In this how should we decide which events are only for tests and which are for tests and production both?

Comment 4

[Harsh]

Is this issue still open and active? I want to take it.

Comment 5

[Kriti Singh]

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #0)

We should have two constants "EVENTS" and "TEST_EVENTS".

Hello, Could you elaborate on which events to put inside the to be created TEST_EVENTS constant? Thanks!

Comment 6

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

To see what events should be inside TEST_EVENTS we need to search the code base [1] and check which ones are only used in tests - i.e. in this test directory: devtools/client/netmonitor/test/

You should also look at this comment and patch:
https://phabricator.services.mozilla.com/D44437#1367914

Basically, all the events that are emitted using emitForTests should be in TEST_EVENTS
But, we should check all of them in the current EVENTS list.
https://searchfox.org/mozilla-central/rev/b2ccce862ef38d0d150fcac2b597f7f20091a0c7/devtools/client/netmonitor/src/constants.js#95

Honza

Comment 7

[manas]

So, if the events are either present in tests or are emitted using emitForTests, then they should be added to TEST_EVENTS, else they should be left in EVENTS and then the corresponding callers should also be changed (example: this.emitForTests(EVENTS.NETWORK_EVENT, actor); should be changed to this.emitForTests(TEST_EVENTS.NETWORK_EVENT, actor);). Is that what needs to be done?

Comment 8

[Kriti Singh]

Attachment: Bug 1580155: Rename event constants used only for tests r=Honza.

Comment 9

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

(In reply to manas from comment #7)

So, if the events are either present in tests

Only in tests ...

or are emitted using emitForTests, then they should be added to TEST_EVENTS, else they should be left in EVENTS and then the corresponding callers should also be changed (example: this.emitForTests(EVENTS.NETWORK_EVENT, actor); should be changed to this.emitForTests(TEST_EVENTS.NETWORK_EVENT, actor);). Is that what needs to be done?

Correct

I'll look a the patch in Phab, thanks!

Honza

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5320b4784ee7
Rename event constants used only for tests r=Honza.

Comment 11

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/5320b4784ee7