Closed
Bug 906081
Opened 11 years ago
Closed 11 years ago
Allow robocop and xpcshell tests to be skipped/failed depending on Android OS version
Categories
(Testing :: XPCShell Harness, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla27
People
(Reporter: gbrown, Assigned: gbrown)
References
Details
Attachments
(3 files, 1 obsolete file)
1.76 KB,
patch
|
ted
:
review+
jmaher
:
feedback+
|
Details | Diff | Splinter Review |
2.23 KB,
patch
|
ted
:
review+
jmaher
:
feedback+
|
Details | Diff | Splinter Review |
1.78 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
This is a follow-up to https://bugzilla.mozilla.org/show_bug.cgi?id=902081#c4. In that bug, we have identified one test that fails for a well understood reason on Android 2.2 but passes on all newer versions of Android. Currently all we can do is skip that test on all Android platforms:
skip-if = os == "android"
For this case it would be convenient to have a method of failing or skipping a test based on OS version also.
Comment 1•11 years ago
|
||
This is tricky because at the moment we don't have any way to get this info. mozinfo.json is generated at build time, where the concept is sort of meaningless.
We could make the harness determine this at runtime (whether by querying the target device, or explicitly passed as a harness argument) and set a key in mozinfo as appropriate. We'd probably have to default it to None because I'm not sure the expression evaluator will handle non-existent keys in expressions sanely.
Assignee | ||
Comment 2•11 years ago
|
||
See the patches on bug 897924 for another approach / work-around.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → gbrown
Summary: Allow xpcshell tests to be skipped/failed depending on Android OS version → Allow robocop and xpcshell tests to be skipped/failed depending on Android OS version
Assignee | ||
Comment 3•11 years ago
|
||
Attachment #809461 -
Flags: review?(ted)
Attachment #809461 -
Flags: feedback?(jmaher)
Assignee | ||
Comment 4•11 years ago
|
||
The same capability would be useful in mochitest-robocop (robocop.ini).
Try run underway at https://tbpl.mozilla.org/?tree=Try&rev=7d79c05da6a8 to verify both xpcshell and robocop.
Attachment #809463 -
Flags: review?(ted)
Attachment #809463 -
Flags: feedback?(jmaher)
Comment 5•11 years ago
|
||
Comment on attachment 809461 [details] [diff] [review]
(1) add androidVersion to mozinfo in remote xpcshell tests
Review of attachment 809461 [details] [diff] [review]:
-----------------------------------------------------------------
::: testing/xpcshell/remotexpcshelltests.py
@@ +544,5 @@
> sys.exit(1)
>
> + # Add Android version (SDK level) to mozinfo so that manifest entries
> + # can be conditional on androidVersion.
> + androidVersion = dm.shellCheckOutput(['getprop', 'ro.build.version.sdk'])
what happens if this fails? I would like some type of warning message printed out if it fails or the value looks invalid.
Attachment #809461 -
Flags: feedback?(jmaher) → feedback+
Comment 6•11 years ago
|
||
Comment on attachment 809463 [details] [diff] [review]
(2) add androidVersion to mozinfo in remote mochitests
Review of attachment 809463 [details] [diff] [review]:
-----------------------------------------------------------------
::: testing/mochitest/runtestsremote.py
@@ +569,5 @@
>
> + # Add Android version (SDK level) to mozinfo so that manifest entries
> + # can be conditional on androidVersion.
> + androidVersion = dm.shellCheckOutput(['getprop', 'ro.build.version.sdk'])
> + log.info("Android version: "+str(androidVersion))
maybe make this say "Android sdk version %s, will use this to filter manifests" % androidVersion
Attachment #809463 -
Flags: feedback?(jmaher) → feedback+
Assignee | ||
Comment 7•11 years ago
|
||
Assignee | ||
Comment 8•11 years ago
|
||
Attachment #809847 -
Attachment is obsolete: true
Assignee | ||
Comment 9•11 years ago
|
||
Comment on attachment 809850 [details] [diff] [review]
(3) teach robocop to skip disabled tests
https://tbpl.mozilla.org/?tree=Try&rev=09618ca2cdaf shows this in action.
Attachment #809850 -
Attachment description: WIP (3) teach robocop to skip disabled tests → (3) teach robocop to skip disabled tests
Attachment #809850 -
Flags: review?(jmaher)
Comment 10•11 years ago
|
||
Comment on attachment 809850 [details] [diff] [review]
(3) teach robocop to skip disabled tests
Review of attachment 809850 [details] [diff] [review]:
-----------------------------------------------------------------
that was easy teachings.
Attachment #809850 -
Flags: review?(jmaher) → review+
Comment 11•11 years ago
|
||
Comment on attachment 809461 [details] [diff] [review]
(1) add androidVersion to mozinfo in remote xpcshell tests
Review of attachment 809461 [details] [diff] [review]:
-----------------------------------------------------------------
::: testing/xpcshell/remotexpcshelltests.py
@@ +545,5 @@
>
> + # Add Android version (SDK level) to mozinfo so that manifest entries
> + # can be conditional on androidVersion.
> + androidVersion = dm.shellCheckOutput(['getprop', 'ro.build.version.sdk'])
> + mozinfo.info['androidVersion'] = androidVersion
nit: I think most of the other mozinfo keys we put in use underscores, not camelCase, so maybe android_version?
We don't currently have support for greater-than or less-than comparisons, is that going to be something we need in the future, or will string equality suffice for now?
Attachment #809461 -
Flags: review?(ted) → review+
Comment 12•11 years ago
|
||
Comment on attachment 809463 [details] [diff] [review]
(2) add androidVersion to mozinfo in remote mochitests
Review of attachment 809463 [details] [diff] [review]:
-----------------------------------------------------------------
If we're going to want this in every harness maybe we can move this into mozinfo itself?
Attachment #809463 -
Flags: review?(ted) → review+
Assignee | ||
Comment 13•11 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #11)
> Comment on attachment 809461 [details] [diff] [review]
> > + # Add Android version (SDK level) to mozinfo so that manifest entries
> > + # can be conditional on androidVersion.
> > + androidVersion = dm.shellCheckOutput(['getprop', 'ro.build.version.sdk'])
> > + mozinfo.info['androidVersion'] = androidVersion
>
> nit: I think most of the other mozinfo keys we put in use underscores, not
> camelCase, so maybe android_version?
Yes, absolutely. I thought there were no existing multi-word keys, but now I see for example, 'service_pack'.
> We don't currently have support for greater-than or less-than comparisons,
> is that going to be something we need in the future, or will string equality
> suffice for now?
String equality will suffice for now, but greater-than/less-than would be useful.
Assignee | ||
Comment 14•11 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #12)
> Comment on attachment 809463 [details] [diff] [review]
> (2) add androidVersion to mozinfo in remote mochitests
>
> Review of attachment 809463 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> If we're going to want this in every harness maybe we can move this into
> mozinfo itself?
But then mozinfo would need to know about devicemanager -- that doesn't seem like a good thing.
Assignee | ||
Comment 15•11 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #5)
> Comment on attachment 809461 [details] [diff] [review]
> > + androidVersion = dm.shellCheckOutput(['getprop', 'ro.build.version.sdk'])
>
> what happens if this fails? I would like some type of warning message
> printed out if it fails or the value looks invalid.
If getprop is not found or shellCheckOutput otherwise has a problem executing the command, a DMError is thrown and reported and the test ends immediately.
If getprop succeeds but ro.build.version.sdk does not exist, then we report:
Mochi-Remote INFO | Android version ''; will be used to filter manifests
and continue. I think that might happen on some developer devices (it does not happen on tegras, pandas, the x86 emulator or any of my devices); I think developers would prefer that this be non-fatal, so that tests can still be run even if this property is missing or non-standard.
Assignee | ||
Comment 16•11 years ago
|
||
Comment 17•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/98c568a0e6e7
https://hg.mozilla.org/mozilla-central/rev/61f225dd6932
https://hg.mozilla.org/mozilla-central/rev/04c327ab2215
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in
before you can comment on or make changes to this bug.
Description
•