Closed Bug 636197 Opened 9 years ago Closed 9 years ago

[SeaMonkey] mochitests-5: test_bug625187.html fails since landing

Categories

(Toolkit :: General, defect, major)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla2.0

People

(Reporter: sgautherie, Assigned: sgautherie)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [perma-orange])

Attachments

(1 file, 2 obsolete files)

{
5782 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/prompts/test/test_bug625187.html | Test must run with tab modal prompts enabled.
5783 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/prompts/test/test_bug625187.html | dialog #1: the tabprompt infoTitle element is hidden - got "", expected "true"
5785 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/prompts/test/test_bug625187.html | dialog #3: the tabprompt infoTitle element is hidden - got "", expected "true"
5786 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/prompts/test/test_bug625187.html | dialog #4: the tabprompt infoTitle element is hidden - got "", expected "true"
}
Whiteboard: [perma-orange]
Why is prompts.tab_modal.enabled set to false?

(that's why the test fails)
(In reply to comment #1)

> Why is prompts.tab_modal.enabled set to false?

Why are toolkit tests relying on Firefox specific settings?
http://mxr.mozilla.org/mozilla-central/search?string=prompts.tab_modal.enabled&case=1

> (that's why the test fails)

Obviously.
Oh, I apologize for the blunder. I didn't know we cannot rely on Firefox-specific settings in toolkit tests.
Attachment #514530 - Flags: review?(dolske) → review+
Comment on attachment 514530 [details] [diff] [review]
(Av1) Skip this test when tab modal prompts are not enabled
[Backed out: Comment 6]

http://hg.mozilla.org/mozilla-central/rev/8f8f80356db4
Attachment #514530 - Attachment description: (Av1) Skip this test when tab modal prompts are not enabled → (Av1) Skip this test when tab modal prompts are not enabled [Checked in: Comment 5]
Blocks: SmTestFail
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
This broke the test on all of the platforms on mozilla-central, so I backed it out:
http://hg.mozilla.org/mozilla-central/rev/8b1eceaf5c89

Please test the next revision on the try server before landing.  Thanks!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 514530 [details] [diff] [review]
(Av1) Skip this test when tab modal prompts are not enabled
[Backed out: Comment 6]

(In reply to comment #6)
> This broke the test on all of the platforms on mozilla-central

Sorry, my bad :-(
Attachment #514530 - Attachment description: (Av1) Skip this test when tab modal prompts are not enabled [Checked in: Comment 5] → (Av1) Skip this test when tab modal prompts are not enabled [Backed out: Comment 6]
(In reply to comment #6)
> test the next revision on the try server

Succeeded:
http://tbpl.mozilla.org/?tree=MozillaTry&rev=caad6d294299
Comment on attachment 514700 [details] [diff] [review]
(Av2) Skip test_bug625187.html when tab modal prompts are not enabled, Clean up and document other tests too
[Partially moved to Bug 637231]

Test cleanups are a waste of reviewer time, especially as a followup to a patch that didn't work at all.
Attachment #514700 - Flags: review?(dolske) → review-
Comment on attachment 514700 [details] [diff] [review]
(Av2) Skip test_bug625187.html when tab modal prompts are not enabled, Clean up and document other tests too
[Partially moved to Bug 637231]

(In reply to comment #10)

> Test cleanups are a waste of reviewer time,

Well, not-cleaned-up tests are a waste of contributors time in the first place:
in this case, global variable names from the common file looking as local variable names (silently) confused me (and you);
and unexpected ok() are confusing log readers and bloating results;
etc.

> especially as a followup to a patch that didn't work at all.

First patch didn't work because 'isTabModal' assignment was mistakenly removed, so its value reverted to (default) 'false'.
I'm just trying to help such a mistake not to happen again.
Attachment #514700 - Flags: review?(dolske)
Comment on attachment 514700 [details] [diff] [review]
(Av2) Skip test_bug625187.html when tab modal prompts are not enabled, Clean up and document other tests too
[Partially moved to Bug 637231]

Nothing's changed here. I'll take a working version of the first patch that fixes just the specific problem this bug is about.
Attachment #514700 - Flags: review?(dolske) → review-
Attachment #514700 - Attachment is obsolete: true
Attachment #514700 - Flags: review-
Comment on attachment 515438 [details] [diff] [review]
(Av3) Skip test_bug625187.html when tab modal prompts are not enabled
[Checked in: See comment 15]

>+  // Update test configuration according to application preference.

This comment doesn't really make sense, remove.

r+ with that, assuming it passes a tryserver run.
Attachment #515438 - Flags: review?(dolske) → review+
Comment on attachment 515438 [details] [diff] [review]
(Av3) Skip test_bug625187.html when tab modal prompts are not enabled
[Checked in: See comment 15]

(In reply to comment #14)
> r+ with that, assuming it passes a tryserver run.

http://tbpl.mozilla.org/?tree=MozillaTry&rev=2817688182cd

http://hg.mozilla.org/mozilla-central/rev/7f5bdbca660e
Attachment #515438 - Attachment description: (Av3) Skip test_bug625187.html when tab modal prompts are not enabled → (Av3) Skip test_bug625187.html when tab modal prompts are not enabled [Checked in: See comment 15]
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Comment on attachment 514700 [details] [diff] [review]
(Av2) Skip test_bug625187.html when tab modal prompts are not enabled, Clean up and document other tests too
[Partially moved to Bug 637231]

I filed bug 637231 on part that matters.
Attachment #514700 - Attachment description: (Av2) Skip test_bug625187.html when tab modal prompts are not enabled, Clean up and document other tests too → (Av2) Skip test_bug625187.html when tab modal prompts are not enabled, Clean up and document other tests too [Partially moved to Bug 637231]
(Firefox is still succeeding and)

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1298869798.1298870617.18687.gz&fulltext=1
WINNT 5.2 comm-central-trunk debug test mochitests-5/5 on 2011/02/27 21:09:58
{
5782 INFO TEST-KNOWN-FAIL | /tests/toolkit/components/prompts/test/test_bug625187.html | Test disabled when tab modal prompts are not enabled.
}

V.Fixed
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.