1625651 - Clean up captive portal tests

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P5)

Description

[:prathiksha]

There are certain pieces of code that can be re-used in some captive portal tests[0].

For example -
a) portalDetected[1] can be re-used in browser_captivePortalTabReference.js [2]
b) openCaptivePortalErrorTab[3] can be moved to head.js and re-used in several captive portal tests
c) openCaptivePortalLoginTab[4] can also be moved to head.js and re-used in several captive portal tests

[0] https://searchfox.org/mozilla-central/source/browser/base/content/test/captivePortal
[1] https://searchfox.org/mozilla-central/rev/8526066f548af9ec3ebb462ff73c47ccc183f533/browser/base/content/test/captivePortal/head.js#43
[2] https://searchfox.org/mozilla-central/rev/8526066f548af9ec3ebb462ff73c47ccc183f533/browser/base/content/test/captivePortal/browser_captivePortalTabReference.js#13
[3] https://searchfox.org/mozilla-central/rev/8526066f548af9ec3ebb462ff73c47ccc183f533/browser/base/content/test/captivePortal/browser_captivePortalTabReference.js#23
[4] https://searchfox.org/mozilla-central/rev/8526066f548af9ec3ebb462ff73c47ccc183f533/browser/base/content/test/captivePortal/browser_captivePortalTabReference.js#53

Comment 1

[Amina Kenessova]

Can I work on this bug?

Comment 2

[:prathiksha]

(In reply to Amina Kenessova from comment #1)

Can I work on this bug?

Hi Amina, I'm sorry, I filed this bug for another contributor I am mentoring. I see you are currently working on a different patch that needs changes. LMK once you are done with that and I can find more bugs for you :)

Comment 3

[:prathiksha]

aleja533, are you still working on this? Do you need any help?

Comment 4

[:prathiksha]

Unassigning due to inactivity.

Comment 5

[Sourab]

Attachment: Bug 1625651 - Clean up captive portal tests. r=prathiksha

Comment 6

[:prathiksha]

Unassigning due to inactivity.

Comment 7

[:prathiksha]

Sourab, are you still interested in finishing your patch? I have left some comments on how it can be improved before landing. No pressure if you're not. I just wanted to ask before forwarding it to another contributor. Thanks!

Comment 8

[bhurerenuka14]

Hi, I was going through the submitted patch - and I found a comment "Refactor browser_captivePortal_certErrorUI.js to also use these helper functions and variables introduced in head.js."
May You please elaborate it , I am not able to get it.

Comment 9

[:prathiksha]

(In reply to bhurerenuka14 from comment #8)

Hi, I was going through the submitted patch - and I found a comment "Refactor browser_captivePortal_certErrorUI.js to also use these helper functions and variables introduced in head.js."
May You please elaborate it , I am not able to get it.

We can use the new helper functions and variables introduced in head.js in browser_captivePortal_certErrorUI.js.

Comment 10

[bhurerenuka14]

Attachment: Bug 1625651 - Clean up captive portal tests. r=prathiksha

Comment 11

[bhurerenuka14]

Attachment: Bug 1625651 - Clean up captive portal tests. r=prathiksha

Depends on D109740

Comment 12

[bhurerenuka14]

hello, I have submitted D109740 with all changes, But then I realized that setCaptivePortalLoginState() wasn't removed.
Therefore I submitted another patch D109768 .

Comment 13

[:prathiksha]

Assigning myself temporarily to prevent the Phabricator Automation bot from assigning it to folks submitting patches. This bug is open in general for all contributors I'm mentoring for Outreachy. Feel free to take a stab at it by submitting a patch.

Comment 14

[Claudia Batista [:claubatista]]

Attachment: Bug 1625651 - Clean up captive portal tests. r=prathiksha

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d01fdaca3696
Clean up captive portal tests. r=prathiksha

Comment 16

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/d01fdaca3696