644697 - jsreftests shell test runner throws "Failed to test XUL condition 'Android'"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

When I run the tests like this:

$ python tests/jstests.py --args="-m -j" build-debug/js

I get this error message:
--
Traceback (most recent call last):
  File "tests/jstests.py", line 295, in <module>
    test_list = manifest.parse(OPTIONS.manifest, xul_tester)
  File "js/src/tests/manifest.py", line 107, in parse
    ans += parse(os.path.join(dir, include_file), xul_tester, include_reldir)
  File "js/src/tests/manifest.py", line 107, in parse
    ans += parse(os.path.join(dir, include_file), xul_tester, include_reldir)
  File "js/src/tests/manifest.py", line 131, in parse
    if xul_tester.test(cond):
  File "js/src/tests/manifest.py", line 78, in test
    raise Exception("Failed to test XUL condition '%s'"%cond)
Exception: Failed to test XUL condition 'Android'
--

Ted says this is a regression from bug 637660.

Comment 1

[(not currently active) Ted Mielczarek]

I suspect the fact that this doesn't get run on Tinderbox anywhere means that we can break it and not notice.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

this is a fairly easy fix.  Inside of reftest.js, we do have some hardcoded constants:
http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest.js#404

There are a couple ways to approach this:
1) change the 2 instances of 'Android' in the jsreftest manifest files to be 'xulRuntime.OS == "Android"'
2) add another variable similar to isDebugBuild to manifest.py: http://mxr.mozilla.org/mozilla-central/source/js/src/tests/manifest.py#20.  

I would prefer option 2, but please let me know if option 1 or something else is preferred.

Comment 3

[David Mandelin [:dmandelin]]

(In reply to comment #2)
> this is a fairly easy fix.  Inside of reftest.js, we do have some hardcoded
> constants:
> http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest.js#404
> 
> There are a couple ways to approach this:
> 1) change the 2 instances of 'Android' in the jsreftest manifest files to be
> 'xulRuntime.OS == "Android"'
> 2) add another variable similar to isDebugBuild to manifest.py:
> http://mxr.mozilla.org/mozilla-central/source/js/src/tests/manifest.py#20.  
> 
> I would prefer option 2, but please let me know if option 1 or something else
> is preferred.

Either option is fine with me.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: add in support for Android keyword

bc, feel free to add anybody else to the review chain.

Comment 6

[David Mandelin [:dmandelin]]

Comment on attachment 521602 [details] [diff] [review]
add in support for Android keyword

Formatting nit: s/self.os=="Android"/self.os == "Android"/

Thanks for fixing this.

Comment 7

[Paul Biggar]

(In reply to comment #1)
> I suspect the fact that this doesn't get run on Tinderbox anywhere means that
> we can break it and not notice.

That'll be solved by bug 642299.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=745fdfab4f58