Closed
Bug 1155610
Opened 11 years ago
Closed 11 years ago
Enable gaia integration tests for multiple device type
Categories
(Firefox OS Graveyard :: Gaia, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: chens, Assigned: chens)
References
Details
Attachments
(5 files, 3 obsolete files)
|
46 bytes,
text/x-github-pull-request
|
gaye
:
feedback+
rickychien
:
feedback+
|
Details | Review |
|
46 bytes,
text/x-github-pull-request
|
rickychien
:
review+
kgrandon
:
feedback+
|
Details | Review |
|
59 bytes,
text/x-github-pull-request
|
gaye
:
review+
|
Details | Review |
|
57 bytes,
text/x-github-pull-request
|
evanxd
:
review+
|
Details | Review |
|
46 bytes,
text/x-github-pull-request
|
chens
:
review+
|
Details | Review |
Before we can enable gij running on treeherder, we should able to run integration tests only for tv (or other device type). Current idea is to put tv's integration test under `apps/test/marionette/module_tv_test.js`, and we can fix[1] here to filter out specific device type's test.
[1] https://github.com/mozilla-b2g/gaia/blob/master/bin/gaia-marionette#L59-L81
| Assignee | ||
Updated•11 years ago
|
Assignee: nobody → chens
Comment 1•11 years ago
|
||
| Assignee | ||
Comment 2•11 years ago
|
||
Comment on attachment 8594571 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice > mozilla-b2g:master
Ricky, we are trying to enable gaia integration test for different devices, test files will be under `apps/test/marionette/GAIA_DEVICE_TYPE` folder, and gaia-marionette will generate test file list corresponding to device type. How do you think?
Attachment #8594571 -
Flags: review?(ricky060709)
Comment 3•11 years ago
|
||
Comment on attachment 8594571 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice > mozilla-b2g:master
IMO, to provide different device type folder is fine to me, but I prefer to split out a "phone" folder as same as "tv" ...etc.
Please tell me what you think and reset r? to me again if you decide to adopt my suggestion. =) thanks!
Attachment #8594571 -
Flags: review?(ricky060709)
| Assignee | ||
Updated•11 years ago
|
Attachment #8594571 -
Attachment is obsolete: true
Comment 4•11 years ago
|
||
| Assignee | ||
Comment 5•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
Per offline discussion, I'm going to move gaia integration tests into folder and let GAIA_DEVICE_TYPE decide which device type test should run.
Ricky, could you review changes in `gaia-marionette` part? And one question for you, if I moved `apps/marionette/manifest.ini` into `apps/marionette/DEVICE/manifest.ini`, should there be any problem?
Attachment #8594661 -
Flags: feedback?(ricky060709)
Comment 6•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
It seems more clear!
Attachment #8594661 -
Flags: feedback?(ricky060709) → feedback+
| Assignee | ||
Comment 7•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
App owners, we are going to support gaia integration test based on different device type, that means we can run phone related gij for phone build, tv related gij for tv build and so on.
So here's slightly change in each app, this patch move `apps/*/test/marionette/*_test.js` to `apps/*/test/marionette/phone/*_test.js` and change relative path for its dependency. Would you review this patch and give some feedback? thanks!
Attachment #8594661 -
Flags: review?(timdream)
Attachment #8594661 -
Flags: review?(squibblyflabbetydoo)
Attachment #8594661 -
Flags: review?(sfoster)
Attachment #8594661 -
Flags: review?(rlu)
Attachment #8594661 -
Flags: review?(ricky060709)
Attachment #8594661 -
Flags: review?(m)
Attachment #8594661 -
Flags: review?(kgrandon)
Attachment #8594661 -
Flags: review?(jburke)
Attachment #8594661 -
Flags: review?(gaye)
Attachment #8594661 -
Flags: review?(francisco)
Attachment #8594661 -
Flags: review?(felash)
Attachment #8594661 -
Flags: review?(drs)
Attachment #8594661 -
Flags: review?(dkuo)
Attachment #8594661 -
Flags: review?(dflanagan)
Attachment #8594661 -
Flags: review?(arthur.chen)
Attachment #8594661 -
Flags: review?(alive)
Comment 8•11 years ago
|
||
I think we might not need to change patches. Could we just add new folders for TV tests?
For example, add a test file in `apps/system/test/marionette/tv/tv_works_well_test.js`.
Then, we just need to add a new command like `make test-integration DEVICE=TV` to run the tests.
How do you think?
Updated•11 years ago
|
Flags: needinfo?(chens)
| Assignee | ||
Comment 9•11 years ago
|
||
I got the same idea as you, attachment 8594571 [details] [review] is it. But after offline discussion with Ricky, it would be more clear to move different device type test into folder, and it would be better change shell script in `gaia-marionette`.
Flags: needinfo?(chens)
Comment 10•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
r+ for gaia-marionette part again!
Attachment #8594661 -
Flags: review?(ricky060709) → review+
Comment 11•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
r=me if the folder structure has been confirmed.
Thanks.
Attachment #8594661 -
Flags: review?(rlu) → review+
Comment 12•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
r=me for settings app, thanks.
Attachment #8594661 -
Flags: review?(arthur.chen) → review+
Updated•11 years ago
|
Summary: Enable gaia integration tests for TV → Enable gaia integration tests for multiple device type
Comment 13•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
Looks good from my point of view but I'd like James' approval as well on this, to know whether this goes in the direction he has in mind.
Flags: needinfo?(jlal)
Attachment #8594661 -
Flags: review?(felash) → review+
Comment 14•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
I definitely want to see what Gareth/James have to say here - as I'm more concerned about the 'vision' than the how.
Personally I think it might be better to specify a device type per test if possible. Let's see what they say though.
Comment 15•11 years ago
|
||
I'm not convinced this is the right way to go. It assumes that all existing tests only apply to a single device type, when in fact, many of them probably apply to multiple types. Indeed, until we write special code in each app for handling various devices, *all* the tests will be device-agnostic.
I think it would make a lot more sense to do this gradually as apps write device-specific code. For instance, tests for a phone could run apps/MY_APP/test/integration/*.js and apps/MY_APP/test/integration/phone/*.js (but not apps/MY_APP/test/integration/tablet/*.js). Then, if and when your app adds some phone-specific features, you could just add the "phone" subdirectory to your tests folder and put the new phone-specific tests there.
Granted, this still isn't perfect, since there's more variation among devices than just "is it a phone or a tablet or a TV?", like whether it has a SIM card (some tablets do), but I think it's better than assuming that all current tests are device-specific.
Comment 16•11 years ago
|
||
> Granted, this still isn't perfect, since there's more variation among
> devices than just "is it a phone or a tablet or a TV?", like whether it has
> a SIM card (some tablets do), but I think it's better than assuming that all
> current tests are device-specific.
That gets to the heart of the issue for me. From the vendor point of view, they just need to know if the code works on a particular device with fixed-in-stone specs before shipping. From our point of view, we need to err on the side of being agnostic and inclusive about device particulars whenever possible. I.e. all tests apply to all devices by default and we have feature-detection (or feature-description) mechanism to skip or configure test suites to ensure meaningful and insightful results for a given device+featureset.
We've had to do this for a long time in browser-land, with tools like has.js(1) being used both for runtime, client-side feature detection and conditional code execution, as well as in packaging scripts for producing pre-optimized builds for that featureset.
If we do want to create these device-type directories, we need to be explicit about the assumptions being made - as these are blurry lines in practice. Some distinctions are real, inherent in the form-factor but others are circumstantial - like having/not having 1+ simcard. Ideally we would run tests using a device/featureset profile that exercises a range of possibilities.
1. https://github.com/phiggins42/has.js/
Comment 17•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
So I just had a quick chat with James over IRC and we think that we should take a different direction here. We see two possible paths forward, there may be more that are not immediately obvious. They are:
1 - A manifest defining device types per test.
2 - Defining the device type within the marionette.client call. Something like: marionette.client({devices: ['phone', 'tv', 'tablet']})
Maybe we should have a vidyo meeting to nail the direction down?
Attachment #8594661 -
Flags: review?(kgrandon) → review-
Comment 18•11 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #17)
> So I just had a quick chat with James over IRC and we think that we should
> take a different direction here. We see two possible paths forward, there
> may be more that are not immediately obvious.
[snip]
> Maybe we should have a vidyo meeting to nail the direction down?
I've thought quite a bit about this sort of problem in my own test framework (sorry, it's C++), and I'd be happy to discuss my solution for it with you and James. I don't want to digress too far off-topic in this bug, so I'll just say here that I prefer to use per-test attributes that indicate whether a test requires a particular feature.
Comment 19•11 years ago
|
||
> 2 - Defining the device type within the marionette.client call. Something
> like: marionette.client({devices: ['phone', 'tv', 'tablet']})
Lets just make sure we define what 'phone', 'tv', 'tablet' mean concretely - in terms of what features we expect - in a json file somewhere. Rather than hard-coding opaque ifdefs where a device type stands in for an unspoken set of features and capabilities. This gives us a path to reconciling forked code and duplicated tests down the road, and a clear way to refine these device type definitions in the future.
Comment 20•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
The very existence of a "phone"/"tablet"/"tv" device type makes me uneasy in a world where the lines between those devices can blur. (How big is a phone before it becomes a tablet? In what ways would an app behave differently under the "tablet" category vs. the "tv" category? Are apps aware of the distinction, or are they using webby media queries?)
Attachment #8594661 -
Flags: review?(m)
Comment 21•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
r- per the above discussion.
Attachment #8594661 -
Flags: review?(squibblyflabbetydoo) → review-
| Assignee | ||
Comment 22•11 years ago
|
||
Agree we should call a meeting, and let me share more context so we can move on.
The initial thought comes from merging tv's system with current gaia system, so we can have one system app supports different device type based on GAIA_DEVICE_TYPE providing in build time. To do so, we may need unit test and integration test covered to prevent breakage, we are targeting it should run tests for both phone and tv build for each pull request, and make sure changes doesn't break any of the devices.
Comment 23•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
Let's discuss before making decision.
Attachment #8594661 -
Flags: review?(alive)
Comment 24•11 years ago
|
||
I just want to say I agree with what has been said until now.
We currently have a manifest already (JSON format), but it's used for blacklisting; I like it that way. Gecko has more a "whitelisting" mechanism using INI manifests, with ways to skip tests if some conditions are met. For some history, we started using it in bug 1039140, but then we stopped using it.
I don't see an obvious solution besides using the same mechanism than gecko. This would have the advantage of consistency for sheriffs. But I don't have a strong opinion on this.
Comment 25•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
Cancelling review pending discussion and/or new plan
Attachment #8594661 -
Flags: review?(sfoster)
Comment 26•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
Agree we should have discussion to narrow down the interface/protocol to filter tests to run, but I am also want to know in how do we setup TV builds for running tests? I did not find that in this patch.
Attachment #8594661 -
Flags: review?(timdream)
Updated•11 years ago
|
Attachment #8594661 -
Flags: review?(dkuo)
Comment 27•11 years ago
|
||
Hey sorry I've been sick for the past couple of days. Just catching up now...
Comment 28•11 years ago
|
||
So initially we wanted each top-level call to marionette() to take a filter object and that worked for marionette-js-runner 0.* (see https://gist.github.com/lightsofapollo/6042786). Initially, we didn't use this feature in gaia though and instead configured the gecko environment (ie desktop, mulet, device, etc) by passing configuration to marionette-js-runner. What this meant was that individual suites and tests wouldn't declare the environment(s) they were intended for and that marionette-js-runner would try to run every call to marionette() on whatever configuration you gave it.
We dropped the environment declaration/filtering feature when we implemented marionette-js-runner 1.0 and now allow you to call marionette-js-runner with a buildapp https://github.com/mozilla-b2g/marionette-js-runner/blob/master/host/index.js#L31 and a runtime https://github.com/mozilla-b2g/marionette-js-runner/blob/master/host/index.js#L38. Is the intention for these tests to run on television devices or an environment like mulet?
Either way, I think that it's wrong to bucket our tests in this way. Like Marcus hinted at above, if the ui is the same between multiple form factors, then there is no reason for this. If the ui is different, then I think we should just have separate apps (one app per dedicated ui). Outside of the testing realm, we are currently working to move more of gaia apps' business and database logic into workers. This will help us build uis for different form factors that use the same backend/api logic. Perhaps we'd have a $GAIA/apps/tablet_calendar app and a $GAIA/apps/tv_calendar app which could each have their own marionette tests.
Currently calendar doesn't have a dedicated tablet ui or a dedicated tv ui, so I wouldn't support moving all of our tests into a phone/ subdirectory.
Flags: needinfo?(jlal)
Comment 29•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
r- based on my comment above
Attachment #8594661 -
Flags: review?(gaye) → review-
Updated•11 years ago
|
Attachment #8594661 -
Flags: review?(jburke) → review?(jrburke)
Comment 30•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
Clearing review for now since seems like more discussion is needed.
Attachment #8594661 -
Flags: review?(jrburke)
| Assignee | ||
Comment 31•11 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #26)
> but I am also want to know in how do we setup TV builds for running tests?
How to setup tv build is not covered in this bug, we will have another bug requesting to run tv gu/gij on treeherder and will discuss how to setup tv build and run that.
We are thinking that it should at least be able to run integration test for tv locally, complete test case and do some try run then ask for running on treeherder.
(In reply to Gareth Aye [:gaye] from comment #28)
> Is the intention for these tests to run on television devices or an environment like mulet?
The intention is to run on b2g-desktop on treeherder, to protect both tv and phone's system.
> Either way, I think that it's wrong to bucket our tests in this way. Like
> Marcus hinted at above, if the ui is the same between multiple form factors,
> then there is no reason for this. If the ui is different, then I think we
> should just have separate apps (one app per dedicated ui). Outside of the
> testing realm, we are currently working to move more of gaia apps' business
> and database logic into workers. This will help us build uis for different
> form factors that use the same backend/api logic. Perhaps we'd have a
> $GAIA/apps/tablet_calendar app and a $GAIA/apps/tv_calendar app which could
> each have their own marionette tests.
Yeah I agree with this, but the situation we got is somewhere in between. The ui on tv is different than ui on phone (system wise ui like dialog, rocketbar, etc.), some of the features are disabled, some of them have different behavior, but they also share part of common features.
So after the merge, system app will look and behave differently on phone and tv. Before we separate backend logic and ui into different apps, how should we support running integration test on different devices for those app having different ui between multiple form factors?
Flags: needinfo?(gaye)
Comment 32•11 years ago
|
||
Thanks for the information, replied inline:
(In reply to Gareth Aye [:gaye] from comment #28)
> So initially we wanted each top-level call to marionette() to take a filter
> object and that worked for marionette-js-runner 0.* (see
> https://gist.github.com/lightsofapollo/6042786). Initially, we didn't use
> this feature in gaia though and instead configured the gecko environment (ie
> desktop, mulet, device, etc) by passing configuration to
> marionette-js-runner. What this meant was that individual suites and tests
> wouldn't declare the environment(s) they were intended for and that
> marionette-js-runner would try to run every call to marionette() on whatever
> configuration you gave it.
>
> We dropped the environment declaration/filtering feature when we implemented
> marionette-js-runner 1.0 and now allow you to call marionette-js-runner with
> a buildapp
> https://github.com/mozilla-b2g/marionette-js-runner/blob/master/host/index.
> js#L31 and a runtime
> https://github.com/mozilla-b2g/marionette-js-runner/blob/master/host/index.
> js#L38. Is the intention for these tests to run on television devices or an
> environment like mulet?
Sorry, I didn't get your point of the above part. Please explain more if possible.
> Either way, I think that it's wrong to bucket our tests in this way. Like
> Marcus hinted at above, if the ui is the same between multiple form factors,
> then there is no reason for this. If the ui is different, then I think we
> should just have separate apps (one app per dedicated ui). Outside of the
> testing realm, we are currently working to move more of gaia apps' business
> and database logic into workers. This will help us build uis for different
> form factors that use the same backend/api logic. Perhaps we'd have a
> $GAIA/apps/tablet_calendar app and a $GAIA/apps/tv_calendar app which could
> each have their own marionette tests.
This is an interesting question.
Should we separate different devices' "UI" as different app??
IIRC, the new gaia architecture tries to separate different view inside of an app, NOT different view as different app. That means that an app should be aware of different device types, like Jim mentioned at comment 15, even we separate different views in different files.
We had made a discussion about different system app for different device at[1]. If we continue do that, it may consumes a lot of maintenance effort. We all agreed that an app should be aware of device type and be able to configure itself at runtime or build-time to meet the requirements of device types. That's the main reason we have this bug and want to merge tv system app to system app. And we just want to maintain the quality of merged code.
As far as I know, TV have different UIs among different apps. Most of the logic are the same, like metadata parsing, storage parsing, internal data structure maintenance, and API/Gecko communications.But TV and phone have totally different UX[2] and Visual[3] specs. So, logics to handle UI are totally differently, like: TV will have more than one homescreen app running at the same time, homescreen app is an overlay on top of other apps, etc. You may argue that why not to separate them as two different app. Yes, we already did that because the homescreen app in TV is totally different than verticalhome. But for some apps which can share 80% codes with different device type. Why should we separate them as an app??
> Currently calendar doesn't have a dedicated tablet ui or a dedicated tv ui,
> so I wouldn't support moving all of our tests into a phone/ subdirectory.
If I am wrong please correct me. According to this, you can accept that we add an TV folder the apps who have TV integration tests. Am I correct?
[1] https://groups.google.com/forum/#!searchin/mozilla.dev.gaia/New$20tv_apps$20folder$20in$20Gaia$20code$20base/mozilla.dev.gaia/7gzVCiuVK9g/ae7V3D0SKrwJ
[2] https://drive.google.com/drive/#folders/0B5RN80W56Ga9WkNBRlZmWUtiaU0
[3] https://drive.google.com/a/mozilla.com/folderview?id=0B2-G3kew1WpXLURNc29vTWl5eUk&usp=sharing
Comment 33•11 years ago
|
||
Oh, I posted wrong link about the discussion. Please ignore the [1] at comment 32. I will continue to find the thread.
Comment 34•11 years ago
|
||
| Assignee | ||
Comment 35•11 years ago
|
||
Comment on attachment 8597148 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-3 > mozilla-b2g:master
How about filter test cases by mocha grep?
We can add device type tag on suite/test name and let mocha decide which to run, so we don't touch existing test cases and it will run as usual, and device specific tests will only run on particular build.
Attachment #8597148 -
Flags: feedback?(gaye)
| Assignee | ||
Updated•11 years ago
|
Attachment #8597148 -
Flags: feedback?(ricky060709)
Attachment #8597148 -
Flags: feedback?(kgrandon)
Comment 36•11 years ago
|
||
Comment on attachment 8597148 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-3 > mozilla-b2g:master
Thanks for the patch. I would like Gareth to look at this. I think it's ok, I do think it would be better to pass in some kind of device profile into the marionette.client constructor or the marionette() call though.
Attachment #8597148 -
Flags: feedback?(kgrandon)
Comment 37•11 years ago
|
||
Comment on attachment 8597148 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-3 > mozilla-b2g:master
yeah, this is the first idea comes out of my mind. If separating tests folder into different device type doesn't make sense for further use case, the scheme of taking mocha grep would be more flexible. In addition, if we don't know how to separate UI / Device logic, we can write all of them in same the test file which means that we could specify such description [phone], [tablet], [tv] within a test and maybe separate them in the future.
Attachment #8597148 -
Flags: feedback?(ricky060709) → feedback+
| Assignee | ||
Comment 38•11 years ago
|
||
We are trying to add more tests for system app on tv, and obviously tests for tv are not going to pass on phone build because ui and interaction are just not the same.
So the idea is to run tests on appropriate build, assuming most of the tests are common among different build, we can let most of common tests running on phone build, device specific test running on device specific build. In that way, we can cover all test cases and running them on its desired target.
Comment 39•11 years ago
|
||
Comment on attachment 8597148 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-3 > mozilla-b2g:master
The mocha grep thing works for me
Flags: needinfo?(gaye)
Attachment #8597148 -
Flags: feedback?(gaye) → feedback+
Comment 40•11 years ago
|
||
Comment on attachment 8594661 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-2 > mozilla-b2g:master
LGTM, thanks!
Attachment #8594661 -
Flags: review?(francisco) → review+
Comment 41•11 years ago
|
||
| Assignee | ||
Comment 42•11 years ago
|
||
This patch comes with attachment 8599027 [details] [review]
| Assignee | ||
Comment 43•11 years ago
|
||
Comment on attachment 8599027 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-4 > mozilla-b2g:master
Gareth, Kevin, how do you think if we do it this way? I'm not sure if we should explicit declare device type for each test so I make it optional.
Attachment #8599027 -
Flags: feedback?(kgrandon)
Attachment #8599027 -
Flags: feedback?(gaye)
| Assignee | ||
Updated•11 years ago
|
Attachment #8599028 -
Flags: feedback?(gaye)
Comment 44•11 years ago
|
||
Comment on attachment 8599027 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-4 > mozilla-b2g:master
I think the `devices` configuration as suggested is an improvement and I would prefer to go with something like this for now. Thank you!
Attachment #8599027 -
Flags: feedback?(kgrandon) → feedback+
| Assignee | ||
Updated•11 years ago
|
Attachment #8599028 -
Flags: feedback?(gaye) → review?(gaye)
Comment 45•11 years ago
|
||
I'm impressed that you found your way around the marionette-js-runner codebase :). I think that --device-type should be a property of the host (see https://github.com/mozilla-b2g/marionette-js-runner/blob/master/host/index.js).
| Assignee | ||
Comment 46•11 years ago
|
||
Yeah, agree --device-type should be in host, move that but keep other as is. I was thinking device type should filter out test before it tries to add to suite, what do you think?
Flags: needinfo?(gaye)
Updated•11 years ago
|
Attachment #8594661 -
Flags: review?(dflanagan)
| Assignee | ||
Comment 48•11 years ago
|
||
Comment on attachment 8599027 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-4 > mozilla-b2g:master
So I think we will go for second option in comment 17, defining the device type within the test,
it would be like: `test('description', {devices: ['some', 'devices', 'to', 'test']}, function(){...});`.
And second argument (device options) for the test is optional, so we don't have to change existing integration tests, by default it will run for phone build. Once device options is provided, marionette-js-runner will filter out tests and only runs for target device type.
Attachment #8599027 -
Flags: review?(ricky060709)
Attachment #8599027 -
Flags: review?(kgrandon)
| Assignee | ||
Updated•11 years ago
|
Attachment #8594661 -
Attachment is obsolete: true
Attachment #8594661 -
Flags: review?(drs)
Comment 49•11 years ago
|
||
Comment on attachment 8599027 [details] [review]
[gaia] shamenchens:Bug1155610-GijForDevice-4 > mozilla-b2g:master
I think Ricky or Gareth should probably review this. Thanks!
Attachment #8599027 -
Flags: review?(kgrandon)
| Assignee | ||
Updated•11 years ago
|
Attachment #8599027 -
Flags: feedback?(gaye) → review?(gaye)
Updated•11 years ago
|
Attachment #8599027 -
Flags: review?(ricky060709) → review+
| Assignee | ||
Comment 50•11 years ago
|
||
Gareth, James, would you take a look at pull requests to gaia and marionette-js-runner and give some feedback? thanks.
Flags: needinfo?(jlal)
Flags: needinfo?(gaye)
Comment 51•11 years ago
|
||
Comment on attachment 8599028 [details] [review]
Pull request to marionette-js-runner
Nice work. I will land and downstream to gaia tomorrow if travis build is green (I just retriggered -- mozilla-download had failed for some reason)
Flags: needinfo?(gaye)
Attachment #8599028 -
Flags: review?(gaye) → review+
| Assignee | ||
Comment 53•11 years ago
|
||
Rebase to latest version and got green build on travis: https://travis-ci.org/mozilla-b2g/marionette-js-runner/builds/62647955
Comment 54•11 years ago
|
||
https://github.com/mozilla-b2g/gaia/pull/29784
I landed this https://github.com/mozilla-b2g/gaia/commit/1d533e6f1acc4008c000e50750f8dedf1d39027e
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: needinfo?(jlal)
Flags: needinfo?(gaye)
Resolution: --- → FIXED
| Assignee | ||
Comment 55•11 years ago
|
||
We still need to land https://github.com/mozilla-b2g/marionette-js-runner/pull/71 on marionette-js-runner and update node_modules in gaia to fix it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 56•11 years ago
|
||
| Comment hidden (obsolete) |
| Assignee | ||
Updated•11 years ago
|
Attachment #8607320 -
Attachment is obsolete: true
Attachment #8607320 -
Flags: review?(evanxd)
| Assignee | ||
Comment 58•11 years ago
|
||
Comment on attachment 8607318 [details] [review]
[gaia-node-modules] shamenchens:Bug1155610-BumpMarionetteJsRunnerVersion > mozilla-b2g:master
Evan, could you help to review this? thanks.
Attachment #8607318 -
Flags: review?(evanxd)
Comment 59•11 years ago
|
||
Comment on attachment 8607318 [details] [review]
[gaia-node-modules] shamenchens:Bug1155610-BumpMarionetteJsRunnerVersion > mozilla-b2g:master
Hi chens,
Good work.
The gaia node module reversion is `4006649e0726f1ef6a5ff8a6a4b8428d6ceb37f3`[1].
[1]: https://github.com/mozilla-b2g/gaia-node-modules/commits/master
Attachment #8607318 -
Flags: review?(evanxd) → review+
| Assignee | ||
Comment 60•11 years ago
|
||
Land on marionette-js-runner: https://github.com/mozilla-b2g/marionette-js-runner/commit/0309c4e2a6fc344ffbb6de289145a89b84f691f9
Land on gaia-node-module: https://github.com/mozilla-b2g/gaia-node-modules/commit/4006649e0726f1ef6a5ff8a6a4b8428d6ceb37f3
Update package.json and gaia_node_modules.revision in bug 1166148: https://github.com/mozilla-b2g/gaia/commit/1b9a0a62de833a8f7b6d4b9caf03998366f2be20
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Comment 61•11 years ago
|
||
Hey have you guys seen our dev-gaia conversation about moving jsmarionette development to gaia? Please go ahead and open a pull request and land your work there. I'm planning on shutting down the other repositories like mozilla-b2g/marionette-js-runner later this week (see bug 1165492)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 62•11 years ago
|
||
Thread in google groups => https://groups.google.com/forum/#!topic/mozilla.dev.gaia/Uik6Eb-jzyk
Comment 63•11 years ago
|
||
| Assignee | ||
Comment 64•11 years ago
|
||
Comment on attachment 8607887 [details] [review]
[gaia] shamenchens:Bug1155610-JsMarionetteForDevice > mozilla-b2g:master
carry over r+
Attachment #8607887 -
Flags: review+
| Assignee | ||
Comment 65•11 years ago
|
||
Land jsmarionette on master: https://github.com/mozilla-b2g/gaia/commit/e6d62cf9b768cd2a2d9cb694149205a8b2415d78
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
| Assignee | ||
Updated•11 years ago
|
Attachment #8599027 -
Flags: review?(gaye)
You need to log in
before you can comment on or make changes to this bug.
Description
•