Closed
Bug 943326
Opened 11 years ago
Closed 7 years ago
run-if = os == 'b2g' does not work in b2g-desktop
Categories
(Testing :: General, defect, P2)
Tracking
(Not tracked)
RESOLVED
INVALID
People
(Reporter: ferjm, Unassigned)
Details
(Keywords: ateam-b2g-task)
Attachments
(1 file)
928 bytes,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
|run-if = os == 'b2g'| does not work in b2g-desktop as the OS_TARGET value is the one corresponding to the host machine. It would be great if we could add a mention to that in the docs [1]. The alternative in this case is to use |run-if = appname == 'b2g'|
[1] https://ci.mozilla.org/job/mozilla-central-docs/Build_Documentation/mozinfo.html#mozinfo-attributes
Comment 1•11 years ago
|
||
ahal, can we just document what is needed here and move on?
I clicked on the link above and it resulted in a 404.
Flags: needinfo?(ahalberstadt)
Comment 2•11 years ago
|
||
I think the expected behaviour here would be for b2g desktop to be included in os == b2g. I believe the only reason it is the way it is now was because b2g desktop didn't exist (or wasn't used in automation) back when this code was written. I'd like ted's opinion though.
I think the only thing that could be affected by this change would be vaibhav's work on b2g desktop mochitest manifests, and it looks like he doesn't use 'os' at all. Joel, do you have any reasons not to do this?
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Attachment #8385340 -
Flags: review?
Flags: needinfo?(ahalberstadt) → needinfo?(jmaher)
Updated•11 years ago
|
Attachment #8385340 -
Flags: review? → review?(ted)
Comment 3•11 years ago
|
||
I would be fine doing this. We could adjust the patch as it hasn't landed yet (waiting on a skip-if for directories right now)
Flags: needinfo?(jmaher)
Comment 4•11 years ago
|
||
I don't think the patch would need to adjusted.. it would just be a cosmetic change (using 'os' instead of 'buildapp')
Comment 5•11 years ago
|
||
I'm really not sure here. I guess you can still differentiate via toolkit, which makes this not terrible either way, but B2G is sort of a weird case, in that it's an OS but also we have desktop builds. I guess if people's expectations are that "os == 'b2g'" matches B2G on any platform, then that's what we ought to do.
Comment 6•11 years ago
|
||
I guess it boils down to this: when using skip-if = os == 'linux', do you also mean to disable the test in b2g desktop? I can see cases where people might inadvertently disable tests on b2g desktop. That being said, I can also see people inadvertently disabling b2g desktop tests when using os == 'b2g' too. I think the takeaway here is that 'os' might not be a very good filter to use when disabling tests.
Maybe instead of lumping it under b2g, we should create a new 'b2gdt' os?
Comment 7•11 years ago
|
||
Comment on attachment 8385340 [details] [diff] [review]
include b2g desktop in os == b2g
Review of attachment 8385340 [details] [diff] [review]:
-----------------------------------------------------------------
This is kind of terrible any way we slice it, so whatever you think is best.
Attachment #8385340 -
Flags: review?(ted) → review+
Updated•11 years ago
|
Keywords: ateam-b2g-task
Updated•11 years ago
|
Priority: -- → P2
Comment 8•11 years ago
|
||
I'm honestly not sure if taking this is a good idea or not. On one hand os == 'Android' for b2g is unintuitive, on the other, people are used to that since that is how it works in the build system. I'm unassigning myself for now..
Assignee: ahalberstadt → nobody
Status: ASSIGNED → NEW
B2G is gone.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•