Closed Bug 728626 Opened 9 years ago Closed 9 years ago

(non-)Firefox tests should not use 'about:robots' which is Firefox specific

Categories

(Testing :: Mochitest, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla15

People

(Reporter: sgautherie, Assigned: raymondlee)

References

()

Details

(Whiteboard: [meta])

Attachments

(1 file, 3 obsolete files)

Take SeaMonkey as an example:
*Core tests are perma-orange.
*Firefox tests are harder to port.
Depends on: 728628
Depends on: 728997
Depends on: 728999
Depends on: 729281
No longer depends on: 728999
I think I'll pass on this one. I've got a full plate right now, and I'm not focusing in this area -- mark
Whiteboard: [meta] → [good first bug][mentor=sgautherie][lang=js] [meta]
Attached patch v1 (obsolete) — Splinter Review
I have updated those robots config in http://mxr.mozilla.org/comm-central/search?string=about%3Arobots&case=1&find=test, except the below:

/mozilla/browser/components/tabview/test/browser_tabview_bug610242.js
line 32 -- let robots = win.gBrowser.loadOneTab("about:robots", bg);
line 82 -- check(robots, "about:robots", true);

This test requires a "about:" page with icon and I couldn't find a about page with icon in seamonkey.  I can either that check in the test or leave it as it is. What do you think?
Assignee: nobody → raymond
Status: NEW → ASSIGNED
Attachment #620608 - Flags: review?(sgautherie.bz)
(In reply to Raymond Lee [:raymondlee] from comment #2)
> /mozilla/browser/components/tabview/test/browser_tabview_bug610242.js
> 
> This test requires a "about:" page with icon and I couldn't find a about
> page with icon in seamonkey.

An 'about:*' page with a favicon?

From SM 2.12a1 about:about, it looks like there is 4 of them:
about:addons ("but" inner content tries to access services.addons.mozilla.org :-/)
about:data (but it is SeaMonkey specific :-()
about:sync-log
about:sync-tabs

And there is more of them, like about:sessionrestore.

Could you try to use one of these?
Comment on attachment 620608 [details] [diff] [review]
v1

Review of attachment 620608 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/test/browser_tabMatchesInAwesomebar.js
@@ +133,5 @@
>          gBrowser.removeTab(tabToKeep);
>          ensure_opentabs_match_db(nextStep);
>        });
>      }, true);
> +    tab.linkedBrowser.loadURI('about:mozilla');

Nit: while here, use '"'.

::: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_zoom.js
@@ +60,4 @@
>  
> +    let mozillaBrowser = gBrowser.getBrowserForTab(tabMozilla);
> +    mozillaBrowser.addEventListener("load", function () {
> +      mozillaBrowser.removeEventListener("load", arguments.callee, true);

Nit: while here, give the listener a name instead of using arguments.callee.

::: browser/components/tabview/test/browser_tabview_bug586553.js
@@ +23,5 @@
>    
>    contentWindow = document.getElementById("tab-view").contentWindow;
>    
>    originalTab = gBrowser.selectedTab;
> +  newTabs = [gBrowser.addTab("about:cache"), gBrowser.addTab("about:mozilla"), gBrowser.addTab("about:license")];

Nit: fine, yet is there any reason to choose about:cache rather than a more usual page like about:[rights] or the like?

::: browser/components/tabview/test/browser_tabview_bug650573.js
@@ +83,2 @@
>    let tab1 = {
> +    entries: [{url: "about:cache"}],

Nit: fine, yet is there any reason to choose about:cache rather than a more usual page like about:[rights] or the like?
Attachment #620608 - Flags: review?(sgautherie.bz) → feedback+
Attached patch v2 (obsolete) — Splinter Review
Passed Try
https://tbpl.mozilla.org/?tree=Try&rev=c569dc435e95
Attachment #620608 - Attachment is obsolete: true
Attachment #625637 - Flags: review?(sgautherie.bz)
Comment on attachment 625637 [details] [diff] [review]
v2

Review of attachment 625637 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/tabview/test/browser_tabview_bug586553.js
@@ +28,3 @@
>  
>    is(originalTab._tPos, 0, "Original tab is in position 0");
> +  is(newTabs[0]._tPos, 1, "Cache is in position 1");

s/Cache/Rights/

@@ +50,5 @@
>    is(moves, 1, "Only one move should be necessary for this basic move.");
>  
>    is(newTabs[2]._tPos, 0, "License is in position 0");
>    is(originalTab._tPos, 1, "Original tab is in position 1");
> +  is(newTabs[0]._tPos, 2, "Cache is in position 2");

s/Cache/Rights/
Attachment #625637 - Flags: review?(sgautherie.bz) → feedback+
Attached patch v3 (obsolete) — Splinter Review
Attachment #625637 - Attachment is obsolete: true
Attachment #625848 - Flags: review?
Attachment #625848 - Flags: review? → review?(sgautherie.bz)
Attachment #625848 - Flags: review?(ttaubert)
Attachment #625848 - Flags: review?(sgautherie.bz)
Attachment #625848 - Flags: feedback+
Attachment #625848 - Flags: review?(ttaubert) → review+
Comment on attachment 625848 [details] [diff] [review]
v3

>Bug 728626 - (non-)Firefox tests should not use 'about:robots' which is Firefox specific try: -m none -u mochitest-o

Remove " try: -m none -u mochitest-o".
Attachment #626044 - Attachment is patch: true
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fceaf8c7e9d
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/6fceaf8c7e9d
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
V.Fixed, per MXR search.
Status: RESOLVED → VERIFIED
Whiteboard: [good first bug][mentor=sgautherie][lang=js] [meta] → [meta]
You need to log in before you can comment on or make changes to this bug.