MarionetteException: Illegal value in CI from test_clock_add_alarm_multiple_times run

VERIFIED FIXED in Firefox 23

Status

--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: stephend, Assigned: mdas)

Tracking

unspecified
mozilla23
Points:
---

Firefox Tracking Flags

(firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
I don't know how reproducible this is, but we saw it in CI:

======================================================================
ERROR: test_clock_add_alarm_multiple_times (test_clock_add_alarm_multiple_times.TestClockAddAlarmMultipleTimes)
Add multiple alarm
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/jenkins/workspace/b2g.unagi.gaia.nightly.ui/gaiatest/tests/clock/test_clock_add_alarm_multiple_times.py", line 40, in test_clock_add_alarm_multiple_times
    self.assertEqual(i, len(self.clock.alarms))
  File "/var/jenkins/workspace/b2g.unagi.gaia.nightly.ui/gaiatest/apps/clock/app.py", line 47, in alarms
    return [self.Alarm(self.marionette, alarm) for alarm in self.marionette.find_elements(*self._all_alarms_locator)]
  File "/var/jenkins/workspace/b2g.unagi.gaia.nightly.ui/.env/local/lib/python2.7/site-packages/marionette_client-0.5.23-py2.7.egg/marionette/marionette.py", line 591, in find_elements
    response = self._send_message('findElements', 'value', **kwargs)
  File "/var/jenkins/workspace/b2g.unagi.gaia.nightly.ui/.env/local/lib/python2.7/site-packages/marionette_client-0.5.23-py2.7.egg/marionette/marionette.py", line 310, in _send_message
    self._handle_error(response)
  File "/var/jenkins/workspace/b2g.unagi.gaia.nightly.ui/.env/local/lib/python2.7/site-packages/marionette_client-0.5.23-py2.7.egg/marionette/marionette.py", line 367, in _handle_error
    raise MarionetteException(message=message, status=status, stacktrace=stacktrace)
TEST-UNEXPECTED-FAIL | test_clock_add_alarm_multiple_times.py TestClockAddAlarmMultipleTimes.test_clock_add_alarm_multiple_times | MarionetteException: Illegal value
----------------------------------------------------------------------
Ran 1 test in 53.035s

Will grab the logcat and attach, once the run is done; wanted to get this on file, first.

Test: https://github.com/mozilla/gaia-ui-tests/blob/master/gaiatest/tests/clock/test_clock_add_alarm_multiple_times.py
App object: https://github.com/mozilla/gaia-ui-tests/blob/master/gaiatest/apps/clock/app.py#L18

    _all_alarms_locator = ('css selector', '#alarms li')
(Reporter)

Updated

5 years ago
Blocks: 801898
(Reporter)

Comment 2

5 years ago
Created attachment 736417 [details]
logcat
I noticed this on desktop too while trying to replicate bug 838607 so makes me suspect a change to transport layer
(Reporter)

Updated

5 years ago
Severity: normal → major
(Reporter)

Comment 4

5 years ago
Malini, mind taking a look at this tomorrow, along with bug 860854?  Both are hitting our automation quite hard (i.e. affecting more tests than I've linked here); thx!
Severity: major → critical
I just saw this error with a run of gaiatest/tests/contacts/test_sort_contacts.py. I fixed a locator in that test and ran it several times. The first 3 passed but the last one failed with the MarionetteException: Illegal value error. Trace is at https://gist.github.com/bobsilverberg/5392338.
Assignee: nobody → mdas
This is due to a bug introduced by bug 736592. If we retrieve an element that's been GC'd, we still try to call XPCNativeWrapper on it and that throws this error. A patch will be up soon.
Created attachment 738065 [details] [diff] [review]
check element before wrapping

Running patch through try first https://tbpl.mozilla.org/?tree=Try&rev=67f1971185ad

I tested this patch locally against the given gaia-ui-test here and it passes.
Comment on attachment 738065 [details] [diff] [review]
check element before wrapping

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

Some drive-by comments.

::: testing/marionette/marionette-elements.js
@@ +71,4 @@
>    */
>    addToKnownElements: function EM_addToKnownElements(element) {
>      for (let i in this.seenItems) {
> +      let foundEl = this.seenItems[i].get();

Should we remove this.seenItems[i] if !foundEl?

@@ -140,5 @@
> -          for(let i in this.seenItems) {
> -            if (this.seenItems[i].get() == val) {
> -              result = {'ELEMENT': i};
> -            }
> -          }

haha, good catch
Comment on attachment 738065 [details] [diff] [review]
check element before wrapping

seems to be going green on try
Attachment #738065 - Flags: review?(jgriffin)
(In reply to Jonathan Griffin (:jgriffin) from comment #8)
> Comment on attachment 738065 [details] [diff] [review]
> check element before wrapping
> 
> Review of attachment 738065 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Some drive-by comments.
> 
> ::: testing/marionette/marionette-elements.js
> @@ +71,4 @@
> >    */
> >    addToKnownElements: function EM_addToKnownElements(element) {
> >      for (let i in this.seenItems) {
> > +      let foundEl = this.seenItems[i].get();
> 
> Should we remove this.seenItems[i] if !foundEl?
> 

Sorry, I missed your comments. I'll change that
Severity: critical → major
Not sure how I managed to change the severity, there, but let's put that back...

Also, I'm trying a patch with the foundEl removal change in it on try: https://tbpl.mozilla.org/?tree=Try&rev=8ad692a58ba1
Severity: major → critical
Created attachment 738127 [details] [diff] [review]
check element before wrapping v2
Attachment #738065 - Attachment is obsolete: true
Attachment #738065 - Flags: review?(jgriffin)
Attachment #738127 - Flags: review?(jgriffin)
Comment on attachment 738127 [details] [diff] [review]
check element before wrapping v2

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

Bah, tested and uploaded the a non-refreshed patch. Don't review yet.
Attachment #738127 - Flags: review?(jgriffin)
Created attachment 738180 [details] [diff] [review]
check element before wrapping v3

fixed error and verified this patch locally against the alarm test.

Testing in try: https://tbpl.mozilla.org/?tree=Try&rev=91abc5d2729a
Attachment #738127 - Attachment is obsolete: true
Attachment #738180 - Flags: review?(jgriffin)
Comment on attachment 738180 [details] [diff] [review]
check element before wrapping v3

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

Thanks mdas.
Attachment #738180 - Flags: review?(jgriffin) → review+
(Reporter)

Updated

5 years ago
Keywords: checkin-needed
(Reporter)

Updated

5 years ago
Keywords: checkin-needed
(Reporter)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-b2g18/rev/b1d335593453
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-firefox21: --- → wontfix
status-firefox22: --- → wontfix
status-firefox23: --- → affected
Target Milestone: --- → mozilla23
(Reporter)

Comment 19

5 years ago
I kicked off a subsequent Unagi engineering build which had this fix, and it's verified FIXED with:

<!-- Mercurial-Information: <project name="releases/mozilla-b2g18" path="gecko" remote="hgmozillaorg" revision="b1d335593453"/> -->
  <!-- Mercurial-Information: <project name="integration/gaia-v1-train" path="gaia" remote="hgmozillaorg" revision="171adc0c8ca1"/> -->
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [leave-open]

Updated

5 years ago
status-firefox23: affected → fixed
(Reporter)

Comment 20

5 years ago
Verified FIXED; thx.
Status: RESOLVED → VERIFIED
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/625540982229
status-b2g18: affected → fixed
status-b2g18-v1.0.1: wontfix → fixed
You need to log in before you can comment on or make changes to this bug.