Closed Bug 494905 Opened 15 years ago Closed 15 years ago

mochitest-browser-chrome: new browser_bug304198.js intermittently fails

Categories

(Firefox :: Address Bar, defect)

Product:
Firefox
Component:
Address Bar
Platform:
All
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 3.6a1

People

(Reporter: sgautherie, Assigned: Gavin)

References

(Depends on 1 open bug,
URL
)

Details

(Keywords: intermittent-failure)

Attachments

(1 obsolete file) 

{
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1243348533.1243353640.3398.gz
Linux mozilla-central unit test on 2009/05/26 07:35:33

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/base/content/test/browser_bug304198.js | gURLBar.value should be testPartialURL (just set) - Got http://example.org/browser/browser/base/content/test/dummy_page.html, expected http://example.org/browser/browser/base/content/test/dummy_page
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/base/content/test/browser_bug304198.js | gURLBar.value should be "" (just set) - Got http://example.org/browser/browser/base/content/test/dummy_page.html, expected
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/base/content/test/browser_bug304198.js | gURLBar.value should be testPartialURL after switching back to partialURLTab - Got http://example.org/browser/browser/base/content/test/dummy_page.html, expected http://example.org/browser/browser/base/content/test/dummy_page
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/base/content/test/browser_bug304198.js | gURLBar.value should be "" after switching back to deletedURLTab - Got http://example.org/browser/browser/base/content/test/dummy_page.html, expected
}
Blocks: 438871
Whiteboard: [orange]
sgautherie - have you seen this bug before (i.e. to know that it's intermittent, and not an actual, recent break?)
Never seen before and previous builds with same rev did not have this bug.
This is basically the same as bug 477631, I think, but we probably can't use the workaround from there. Instead, we could set gBrowser.userTypedValue directly :(
In that case would the first version of the test case be better? It is part of the 2nd patch for bug #304198

https://bug304198.bugzilla.mozilla.org/attachment.cgi?id=379572
That's basically what I was referring to, yes.
Ok, I'll update the test case and send a patch in tonight.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1243358881.1243363339.21069.gz
Linux mozilla-central unit test on 2009/05/26 10:28:01
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1243356358.1243363858.22132.gz
Linux mozilla-central unit test on 2009/05/26 09:45:58
This patch removes the EventUtils.synthesizeKey and related calls in favor of gBrowser.userTypedValue and URLBarSetURI calls so the test will pass for linux. Tested with OpenSuse 11.1.
Are we really testing the right thing by not synthesizing key events though?  If we somehow break key hanlding, this test won't fail like it should.
Also, do we understand /why/ synthesizeKey doesn't work?  I would rather understand the failure here and make sure we don't have a bug on the platform level.
(In reply to comment #10)
> If we somehow break key hanlding, this test won't fail like it should.

I'm not too concerned about that, as we have extensive human testing in that area. I'm concerned about creating tests that will fail for no good reason if we change implementation details, and therefore it would certainly be better to really simulate the user input.

(In reply to comment #11)
> Also, do we understand /why/ synthesizeKey doesn't work?

Nope, we already gave up on that in bug 477631.
Do I need to keep the synthesizeKey approach and resolve the core issue with it not working on linux to get this test to pass? I do agree that it is the better approach, but was under the impression that I needed to go back to setting gBrowser.userTypedValue for this test...
(In reply to comment #12)
> I'm not too concerned about that, as we have extensive human testing in that
> area. I'm concerned about creating tests that will fail for no good reason if
> we change implementation details, and therefore it would certainly be better to
> really simulate the user input.
Human testing is expensive in human time and human resources, and we should try to avoid it with automated tests whenever possible.  Machine time is cheap.


I would much prefer that we understand the issue before jumping to fixes.  I don't see how we gave up on figuring out what was wrong in bug 477631.  I don't see us getting people involved with that issue that would likely know a lot about that code (like Neil Deakin).
(In reply to comment #13)
> Do I need to keep the synthesizeKey approach and resolve the core issue with it
> not working on linux to get this test to pass?

That would be ideal. But first of all, you're not responsible for the
core issue. If you see a way to figure out what the core issue /is/, that would
already be a huge step forward. If you even want to resolve it, nobody will
stop you either :)

(In reply to comment #14)
> (In reply to comment #12)
> > I'm not too concerned about that, as we have extensive human testing in that
> > area. I'm concerned about creating tests that will fail for no good reason if
> > we change implementation details, and therefore it would certainly be better to
> > really simulate the user input.
> Human testing is expensive in human time and human resources, and we should try
> to avoid it with automated tests whenever possible.  Machine time is cheap.

Right, but manual testing happens constantly in this area, you don't have to ask anyone to run a litmus test or something like that. If you break typing in the location bar, you'll know as soon as anyone tries to use a build with your changeset. Apart form that, I would assume that other automated tests cover key handling already -- that's not really the point of this test. Anyway, we all agree that synthesizeKey is preferable, and I definitely would like to understand why it doesn't work reliably, as it will bite us again elsewhere.
Ok thats fair, I'll spend on some time digging around on linux to see what the core issue is, and hopefully how to resolve it.
I ran the browser chrome tests on linux quite a bit last night, and got intermittent failures, as reported. One thing I did notice, and this might be a) known to other besides myself, or b) just a red herring - but it appears that whenever the tests fail, a window with a single button that says "Run [test]" (browser-harness.xul?) appears in the foreground temporarily. Has anyone else seen this? Just thought I would ask those more knowledgeable in this area before going to deep down a rabbit hole.
Can we disable this test for now, or replace the trunk version with the test on the branch? It's preventing checkins of other blocker fixes on m-c.
I pushed the branch test to the trunk to solve the intermittent failures:
https://hg.mozilla.org/mozilla-central/rev/bfb383af1903
Whiteboard: [orange] → [test replaced by branch one] [orange]
Depends on: 495751
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee: nobody → gavin.sharp
Whiteboard: [test replaced by branch one] [orange] → [1.9.2 trunk test replaced by 1.9.1 branch one] [orange]
Target Milestone: --- → Firefox 3.6a1
Depends on: 509295
(In reply to comment #19)
> https://hg.mozilla.org/mozilla-central/rev/bfb383af1903

Backed out in bug 509295.
Whiteboard: [1.9.2 trunk test replaced by 1.9.1 branch one] [orange] → [orange]
Attachment #379822 - Attachment description: Updated test → Updated test [Backout: See comment 20]
Attachment #379822 - Attachment is obsolete: true
(In reply to comment #21)
> Can someone please land this on 1.9.2?

Which "this" exactly?
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: