970409 - References to "android_version" in test manifests cause failures

Status: RESOLVED WONTFIX

(Testing :: General, defect)

Description

[Geoff Brown [:gbrown]]

There is support for using "android_version" in xpcshell and mochitest manifests:

skip-if = os == "android" && android_version == "10"

We have not used this much historically, but I was just going to start using it for manifest changes needed for the Android 2.3 emulator tests.

Now that's broken, by bug 956474 (which seems like a good thing otherwise!):

https://tbpl.mozilla.org/php/getParsedLog.php?id=34424831&full=1&branch=ash#error0

08:35:28     INFO - Calling ['/builds/slave/test/build/venv/bin/python', '-u', '/builds/slave/test/build/tests/xpcshell/runxpcshelltests.py', '--symbols-path=http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/ash-linux/1392045546/firefox-30.0a1.en-US.linux-i686.crashreporter-symbols.zip', '--test-plugin-path=/builds/slave/test/build/application/firefox/plugins', '--manifest=tests/xpcshell/tests/all-test-dirs.list', '/builds/slave/test/build/application/firefox/xpcshell'] with output_timeout 1000
08:35:29     INFO -  Found node at /usr/bin/node
08:35:29     INFO -  Found moz-spdy at /builds/slave/test/build/tests/xpcshell/moz-spdy/moz-spdy.js
08:35:29     INFO -  Found moz-http2 at /builds/slave/test/build/tests/xpcshell/moz-http2/moz-http2.js
08:35:29     INFO -  Traceback (most recent call last):
08:35:29     INFO -    File "/builds/slave/test/build/tests/xpcshell/runxpcshelltests.py", line 1625, in <module>
08:35:29     INFO -      main()
08:35:29     INFO -    File "/builds/slave/test/build/tests/xpcshell/runxpcshelltests.py", line 1621, in main
08:35:29     INFO -      if not xpcsh.runTests(args[0], testdirs=args[1:], **options.__dict__):
08:35:29     INFO -    File "/builds/slave/test/build/tests/xpcshell/runxpcshelltests.py", line 1325, in runTests
08:35:29     INFO -      self.buildTestList()
08:35:29     INFO -    File "/builds/slave/test/build/tests/xpcshell/runxpcshelltests.py", line 800, in buildTestList
08:35:29     INFO -      self.alltests = mp.active_tests(**mozinfo.info)
08:35:29     INFO -    File "/builds/slave/test/build/venv/local/lib/python2.7/site-packages/manifestparser/manifestparser.py", line 1091, in active_tests
08:35:29     INFO -      self.filter(values, tests)
08:35:29     INFO -    File "/builds/slave/test/build/venv/local/lib/python2.7/site-packages/manifestparser/manifestparser.py", line 1060, in filter
08:35:29     INFO -      if parse(condition, **values):
08:35:29     INFO -    File "/builds/slave/test/build/venv/local/lib/python2.7/site-packages/manifestparser/manifestparser.py", line 279, in parse
08:35:29     INFO -      return ExpressionParser(text, values, strict=True).parse()
08:35:29     INFO -    File "/builds/slave/test/build/venv/local/lib/python2.7/site-packages/manifestparser/manifestparser.py", line 266, in parse
08:35:29     INFO -      raise ParseError("could not parse: %s; variables: %s" % (self.text, self.valuemapping))
08:35:29     INFO -  manifestparser.manifestparser.ParseError: could not parse: os == "android" && android_version == "10"; variables: {u'appname': u'firefox', u'tests_enabled': True, 'bits': 32, u'bin_suffix': u'', u'mozconfig': u'/builds/slave/ash-lx-00000000000000000000000/build/.mozconfig', u'wave': True, u'topsrcdir': u'/builds/slave/ash-lx-00000000000000000000000/build', u'toolkit': u'gtk2', u'webm': True, u'crashreporter': True, u'buildapp': u'browser', 'version': 'Ubuntu 12.04', u'datareporting': True, u'debug': False, u'asan': False, u'ogg': True, 'hasNode': True, 'os': u'linux', 'processor': u'x86'}
08:35:29    ERROR - Return code: 1

Comment 1

[(not currently active) Ted Mielczarek]

My suggestion was simply to set android_version = None in mozinfo.py:
http://mxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/mozinfo.py

Then a harness can overwrite that with a sane value since it can only be detected at runtime.

The only issue here would be if we wanted to use this in Mochitest manifests, since currently those rely on being able to evaluate manifest conditionals at build time.

Comment 2

[Geoff Brown [:gbrown]]

Attachment: define android_version in mozinfo

Ted's suggestion seems to work fine: https://tbpl.mozilla.org/?tree=Ash&rev=9260c93ad3a2

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 8373691 [details] [diff] [review]
define android_version in mozinfo

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

Except that it silently breaks for the mochitest case, right?

Comment 4

[(not currently active) Ted Mielczarek]

Mochitest doesn't actually support this key in manifests yet. We will have to hold off adding it there until we've moved all Mochitests to manifests, and can turn off the "filter at build time" logic.

Comment 5

[Geoff Brown [:gbrown]]

We have worked around my original issue using android23.json for now, and I have faith that we will sort out all the issues in time...this bug seems extraneous now.