Closed Bug 780556 Opened 8 years ago Closed 7 years ago

Mozmill test failure remote/restartTests/testDiscoveryPane_UpAndComingModule/test1.js sometime fails with 'aElement is undefined'

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect, P2)

defect

Tracking

(firefox18 disabled, firefox19 disabled, firefox20 disabled, firefox21 disabled, firefox-esr10 wontfix, firefox-esr17 disabled)

RESOLVED DUPLICATE of bug 732353
Tracking Status
firefox18 --- disabled
firefox19 --- disabled
firefox20 --- disabled
firefox21 --- disabled
firefox-esr10 --- wontfix
firefox-esr17 --- disabled

People

(Reporter: vladmaniac, Unassigned)

References

()

Details

(Whiteboard: [mozmill-test-failure][remote][mozmill-test-skipped])

Attachments

(2 files, 7 obsolete files)

Firefox versions: All branches
OS: all
Mozmill version: 1.5.17 
Error report on ci: http://mozmill-ci.blargon7.com/#/remote/report/29fc09ba0c8360d637617903a01672f1
Error log: 
DiscoveryPane_getInstallSource@resource://mozmill/stdlib/securable-module.js -> file:///c:/docume~1/mozilla/locals~1/temp/tmp99_jp9.mozmill-tests/lib/addons.js:1251 testInstallUpAndComingAddon@resource://mozmill/modules/frame.js -> file:///c:/docume~1/mozilla/locals~1/temp/tmp99_jp9.mozmill-tests/tests/remote/restartTests/testDiscoveryPane_UpAndComingModule/test1.js:49 @resource://mozmill/modules/frame.js:591 @resource://mozmill/modules/frame.js:661 @resource://mozmill/modules/frame.js:707 @resource://mozmill/modules/frame.js:540 @resource://mozmill/modules/frame.js:719 @resource://jsbridge/modules/server.js:179 @resource://jsbridge/modules/server.js:183 @resource://jsbridge/modules/server.js:283 
Error: 'aElement is undefined'
Whiteboard: [mozmill-test-failure][remote]
Summary: Mozmill test failure remote/restartTests/testDiscoveryPane_UpAndComingModule/test1.js sometime fails with 'aElement is undefined ' → Mozmill test failure remote/restartTests/testDiscoveryPane_UpAndComingModule/test1.js sometime fails with 'aElement is undefined'
Assignee: nobody → andreea.matei
Status: NEW → ASSIGNED
Assigning myself since Andreea is already actively working on several bugs and I would like this investigated asap. 

We want to re-use this test and unskip as many remote tests as we can.
Assignee: andreea.matei → vlad.mozbugs
This test installs an add-on randomly from the up-and coming list so the first thing I did was to check if all the extensions from the list are installing correctly - no errors were found. 
Currently I am running this particular test on a heavy load environment, but since the error is "aElement" I would really want to enable extension logging for this test and monitor jenkins master see what we get. 

If passing, we should have a report log which would look like this: 

*** LOG addons.xpi: Download started for https://addons.mozilla.org/firefox/downloads/latest/376972/addon-376972-latest.xpi?src=discovery-upandcoming to file /tmp/tmp-y7m.xpi
*** LOG addons.xpi: Download of https://addons.mozilla.org/firefox/downloads/latest/376972/addon-376972-latest.xpi?src=discovery-upandcoming completed.
*** LOG addons.repository: Requesting https://services.addons.mozilla.org/en-US/firefox/api/1.5/search/guid:%7Bd4e0dc9c-c356-438e-afbe-dca439f4399d%7D?src=firefox&appOS=Linux&appVersion=17.0a1
*** LOG addons.xpi: Starting install of https://addons.mozilla.org/firefox/downloads/latest/376972/addon-376972-latest.xpi?src=discovery-upandcoming
*** LOG addons.xpi: Addon {d4e0dc9c-c356-438e-afbe-dca439f4399d} will be installed as an unpacked directory
*** LOG addons.xpi: Install of https://addons.mozilla.org/firefox/downloads/latest/376972/addon-376972-latest.xpi?src=discovery-upandcoming completed.
* enables extension logging which would be very useful for debugging
Attachment #651332 - Flags: review?(hskupin)
Attachment #651332 - Flags: review?(dave.hunt)
Comment on attachment 651332 [details] [diff] [review]
enable extension logging patch v.10

This will not help. If you check the stack of the failure you will see that we fail here:

    47   // Install the addon

    48   var addToFirefox = discovery.getElement({type: "addon_installButton"});

>   49   var currentInstallSource = discovery.getInstallSource(addToFirefox);
Attachment #651332 - Flags: review?(hskupin)
Attachment #651332 - Flags: review?(dave.hunt)
Attachment #651332 - Flags: review-
Yes - the extension page loads lazily in the discovery pane. Its easy to see this manually - just perform the manual steps of the test. 

I am going to check if waiting for the element helps in any way
Attached patch patch v1.1 [backed-out] (obsolete) — Splinter Review
* added waitForElement(addToFirefox); 
* removed existent trailing whitespaces in the patch
Attachment #651332 - Attachment is obsolete: true
Attachment #651654 - Flags: review?(hskupin)
Comment on attachment 651654 [details] [diff] [review]
patch v1.1 [backed-out]

Sounds reasonable. Lets try this out.
Attachment #651654 - Flags: review?(hskupin) → review+
http://hg.mozilla.org/qa/mozmill-tests/rev/4acb7f79b5f6

Lets see how it behaves. Please check the reports in the next couple of days. If we are green, we should backport the patch.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Vlad, please monitor your own fixes. This one is not going to work yet. It's failing again just slightly different:

http://mozmill-ci.blargon7.com/#/remote/report/d87d47fd1034f072b9bece6ee635e911

    elem is undefined
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Henrik Skupin (:whimboo) from comment #9)
> Vlad, please monitor your own fixes. 

I am - please keep in mind that all of us had a national day on the 15th
(In reply to Henrik Skupin (:whimboo) from comment #9)
> Vlad, please monitor your own fixes. This one is not going to work yet. It's
> failing again just slightly different:
> 
> http://mozmill-ci.blargon7.com/#/remote/report/
> d87d47fd1034f072b9bece6ee635e911
> 
>     elem is undefined

Normally I would think the default timeout should be enough here, but since this is intermittent and I cannot reproduce this locally, I can only suggest that we set a bigger timeout.
Attached patch increase timeout v1.0 (obsolete) — Splinter Review
* Adding extra timeout
Attachment #652337 - Flags: review?(hskupin)
Attachment #652337 - Flags: review?(dave.hunt)
If this was a timeout I would expect the failure to be "Timeout exceeded for waitForElement..."

I suspect perhaps that discovery.getElement({type: "addon_installButton"}) is returning undefined.
(In reply to Dave Hunt (:davehunt) from comment #13)
> I suspect perhaps that discovery.getElement({type: "addon_installButton"})
> is returning undefined.

Most likely. Especially nodeCollector doesn't work with dynamically created pages which dynamically inject nodes. This could be the reason here. Sorry that I missed that.

At some point we need that feature in nodeCollector so I would propose we add that. Vlad, mind filing a bug on it?
Comment on attachment 651654 [details] [diff] [review]
patch v1.1 [backed-out]

Backed out this patch due to it's a noop:
http://hg.mozilla.org/qa/mozmill-tests/rev/fad47411458c
Attachment #651654 - Attachment description: patch v1.1 → patch v1.1 [backed-out]
Attachment #651654 - Attachment is obsolete: true
Depends on: 783490
Comment on attachment 652337 [details] [diff] [review]
increase timeout v1.0

This is not going to work. Questions beside. Is the install button visible when the pane has been loaded or do you have to hover over an item?
Attachment #652337 - Attachment is obsolete: true
Attachment #652337 - Flags: review?(hskupin)
Attachment #652337 - Flags: review?(dave.hunt)
Status: REOPENED → ASSIGNED
Added bug 783490 and marked as blocking for this one. 
Henrik, can you please add the required whiteboard entries as I'm not sure about which would be needed there.
(In reply to Henrik Skupin (:whimboo) from comment #16)
> Comment on attachment 652337 [details] [diff] [review]
> increase timeout v1.0
> 
> This is not going to work. Questions beside. Is the install button visible
> when the pane has been loaded or do you have to hover over an item?

No hovering, but you have to wait for the element. Its clearly visible that its loaded lazily in the page.
We cold go with a sleep, but I would say lets disable this test for now until the nodeCollector bug has been fixed.
* disabling the test as to Henrik's suggestion
Attachment #652704 - Flags: review?(hskupin)
Attachment #652704 - Flags: review?(dave.hunt)
* esr branch is affected looking at the flags, so adding a patch for esr also
Attachment #652705 - Flags: review?(hskupin)
Attachment #652705 - Flags: review?(dave.hunt)
Comment on attachment 652705 [details] [diff] [review]
[mozilla-esr10][skip patch] disable test v1.0

>Bug 780556 - Disable the test due to 'elem is undefined' error. r=hskupin, r=davehunt

This should be dhunt. Other than that I don't see any difference between this patch and attachment 652704 [details] [diff] [review]. Is a patch needed for ESR?
Attachment #652705 - Flags: review?(hskupin)
Attachment #652705 - Flags: review?(dave.hunt)
Attachment #652705 - Flags: review-
Comment on attachment 652704 [details] [diff] [review]
[default][skip patch] disable test v1.0

Please also disable the test in the manifest file.
Attachment #652704 - Flags: review?(hskupin)
Attachment #652704 - Flags: review?(dave.hunt)
Attachment #652704 - Flags: review-
* fixed to disable the test in the manifest also
Attachment #652704 - Attachment is obsolete: true
Attachment #653257 - Flags: review?(hskupin)
Attachment #653257 - Flags: review?(dave.hunt)
Attachment #652705 - Attachment is obsolete: true
Attachment #653258 - Flags: review?(dave.hunt)
Attachment #653257 - Attachment description: [default][skipa patch] disable test v1.1 → [default][skip patch] disable test v1.1
Attachment #653258 - Flags: review?(hskupin)
Comment on attachment 653257 [details] [diff] [review]
[default][skip patch] disable test v1.1

>+// XXX: Bug 780556
>+//      Skip because of 'elem is undefined' error

Why such a comment? All the information we need is in the skip message already.
Attachment #653257 - Flags: review?(hskupin)
Attachment #653257 - Flags: review?(dave.hunt)
Attachment #653257 - Flags: review-
Comment on attachment 653258 [details] [diff] [review]
[mozilla-esr10][skip patch] disable test v1.1

Same for the esr10 patch.
Attachment #653258 - Flags: review?(hskupin)
Attachment #653258 - Flags: review?(dave.hunt)
Attachment #653258 - Flags: review-
* removed comment 

I agree its redundant, but used it for consistency. We used to have a practice in adding an //XXX: with the bug details before skipping the test.
Attachment #653257 - Attachment is obsolete: true
Attachment #653677 - Flags: review?(hskupin)
Attachment #653677 - Flags: review?(dave.hunt)
* killed redundant comment
Attachment #653258 - Attachment is obsolete: true
Attachment #653678 - Flags: review?(hskupin)
Attachment #653678 - Flags: review?(dave.hunt)
Attachment #653677 - Flags: review?(hskupin)
Attachment #653677 - Flags: review?(dave.hunt)
Attachment #653677 - Flags: review+
Comment on attachment 653677 [details] [diff] [review]
disable test v1.2 [checked-in]

Just a note, please be more specific with the commit messages in the future. Means what the patch is doing. Using the bug summary is not always a good idea.

http://hg.mozilla.org/qa/mozmill-tests/rev/6b74e72c1080 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/ed13a0dafd1b (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/2f4760268961 (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/19d7a25e2e09 (release)
Attachment #653677 - Attachment description: [default][skipa patch] disable test v1.2 → disable test v1.2 [checked-in]
Comment on attachment 653678 [details] [diff] [review]
disable test v1.2 [mozilla-esr10][checked-in]

http://hg.mozilla.org/qa/mozmill-tests/rev/a7b933926e9c (esr10)
Attachment #653678 - Attachment description: [mozilla-esr10][skip patch] disable test v1.2 → disable test v1.2 [mozilla-esr10][checked-in]
Attachment #653678 - Flags: review?(hskupin)
Attachment #653678 - Flags: review?(dave.hunt)
Attachment #653678 - Flags: review+
Whiteboard: [mozmill-test-failure][remote] → [mozmill-test-failure][remote][mozmill-test-skipped]
Status: ASSIGNED → NEW
Priority: -- → P2
Assignee: vlad.mozbugs → nobody
Cosmin, will this be fixed with the patch on bug 732353?
Flags: needinfo?(cosmin.malutan)
Yes, it will, I'll mark this as dependent.
Depends on: 732353
Flags: needinfo?(cosmin.malutan)
No need to add this as dependency. We can dupe against bug 732353.
Status: NEW → RESOLVED
Closed: 8 years ago7 years ago
No longer depends on: 732353
Resolution: --- → DUPLICATE
Duplicate of bug: 732353
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.