Closed Bug 1027475 Opened 10 years ago Closed 8 years ago

Use Karma to run unit tests in Gaia project

Categories

(Firefox OS Graveyard :: Gaia::TestAgent, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::TestAgent
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: rickychien, Assigned: fbukevin, Mentored)

References

Details

Attachments

(7 files)

my setup.js
5.25 KB, application/x-javascript
Details
First Test Result
27.97 KB, text/plain
Details
video_test_result
47.29 KB, text/plain
Details
success.png
230.67 KB, image/png
Details
failed.png
154.61 KB, image/png
Details
video_test_success.png
67.53 KB, image/png
Details
all_test.txt
2.21 KB, text/plain
Details 
Karma[1] is a powerful JavaScript testing environment that is pretty configurable for a wide range of well known framework such as Mocha, Jasmine, Qunit, Sinon, Chai and so on and test your JavaScript on real browser or on NodeJS. Also provided other useful plugins to enhance or customize.

List advantages of Karma here:

  * test code in real browsers.
  * test code in multiple browsers (desktop, mobile, tablets, etc.).
  * execute your tests locally during development.
  * execute your tests on a continuous integration server (Travis, Jenkins).
  * execute your tests on every save.
  * use Istanbul to automagically generate coverage reports.
  * use RequireJS for your source files.

Karma does every jobs which our test-agent does too. It's open source and seems very active now, and moreover, it attracts a lot of developers to contribute on it.

Another we want is trying to make our testing framework more suitable for every developer who just wants to test their own FFOS app. So after downloading our testing tool, developers could test their own app without setting up a bunch of configurations.

I think it's a good chance to try to investigate Karma for our new testing environment. Making our life more easy since we don't need to reinvent the wheel.

[1] http://karma-runner.github.io/
Evan and I proposed this project in TSOC program (just like GSOC). We are going to become mentors in this project. Our participant student Veck will help us investigate the possibility of using Karma in Gaia during this summer vacation.

I think that would be good to hear someone have other suggestions.
Flags: needinfo?(jlal)
Flags: needinfo?(felash)
I have no objections to using Karma or some other popular test runner though ideally we need to run each file in isolation.
Flags: needinfo?(jlal)
Assignee: nobody → fbukevin
Mentor: evanxd, ricky060709
Provide isolation of each set of tests in Karma has discussed in [1]. We are going to survey about isolation in Karma or maybe we need to patch Karma for this new feature.

[1] https://github.com/karma-runner/karma/issues/412
In the TSOC project, in our first step, we will try to run unit tests existed in Gaia with Karam. It will help us to figure out and fix the issues of using Karma in Gaia. Secondly, we would like to patch Karam to have the isolation feature.

Then we could try to use Karam in Gaia, and it means that we might not need the js-test-agent anymore.
Hi Veck,

The unit test files is in the `apps/[app]/test/unit` folder for each app. For example, the files for calendar app is in [1]. And the pattern of the file name is like `xxx_test.js`.

In the first step, we could figure out how to run these test files with Karma.

[1] https://github.com/mozilla-b2g/gaia/tree/master/apps/calendar/test/unit
You could refer to [1] to know how to run the unit tests for Gaia.

[1] https://github.com/mozilla-b2g/gaia#unit-tests
I have no issue with this either. If we can move from something we maintain ourself to something maintained by an extenral team, I'd be happy. I'm not sure it will be easy though :)
Flags: needinfo?(felash)
Hello everyone! 
I will start to study how to run the unit tests for Gaia first.
Wish that we could have a good collaboration. :)
Weekly work(2014/6/20~2014/6/27):
Install Karma and study how to start it with the official tutorial：
http://karma-runner.github.io/0.12/intro/installation.html

The problem I met is that after create the karma.conf.js with unit test files location, I always get error message like the following said that I didn't defined require.
 ~ -->karma start karma.conf.js 
INFO [karma]: Karma v0.12.16 server started at http://localhost:9876/
INFO [launcher]: Starting browser Chrome
INFO [Chrome 35.0.1916 (Mac OS X 10.9.3)]: Connected on socket 1MCe73jxMowOFW2nJExn with id 4732356
Chrome 35.0.1916 (Mac OS X 10.9.3) ERROR
  Uncaught ReferenceError: require is not defined
  at /Users/veck/gaia/apps/video/test/unit/thumbnail_date_group_test.js:6

Chrome 35.0.1916 (Mac OS X 10.9.3) ERROR
  Uncaught ReferenceError: require is not defined
  at /Users/veck/gaia/apps/video/test/unit/thumbnail_item_test.js:6
Our test agent use a custom require function to load source file in each test.

To solve this problem, we could add a setup.js file before running tests in karma. The setup.js is going to define some utils functions like as require, requireApp...etc. And the require function is defined at [1]. So, I think the first step is that you should try to include [1] into karma and make it work.

[1] https://github.com/mozilla-b2g/js-test-agent/blob/master/lib/test-agent/loader.js
Note that we want to deprecate requireApp in bug 978103.

Also bug 943163 modifies "require", so once it lands, please don't forget to move these changes too.
Thanks Ricky!
I try to move code in loader.js into setup.js, and load it into karma. And I successfully make 'require' be called. I'm trying to implement requireApp in setup.js.

I found require, requireApp, ...etc in file of gaia project:
1. ~/gaia/dev_apps/test-agent/common/test/helper.js => defines requireApp, requireElement, requireCommon
2. ~/gaia/dev_apps/test-agent/common/vendor/test-agent/test-agent.js => only defines require function like https://github.com/mozilla-b2g/js-test-agent/blob/master/lib/test-agent/loader.js

I wonder that why should we make a setup.js instead of loading those definition files in karma.conf.js? Is because we plan to discard using test-agent and replace it with Karma, so we have to move those require* function into setup.js for Karma to use?

On the other hand, would you recommend me to use RequireJS to load those files?

Finally, thanks to Julien's remind!
Hi Veck,

For the setup.js, I think using setup.js is just is practice of writing code.

Could you share the repo here?
Then I could response you clearer.

Thanks.
Using RequireJS is a good way to make the structure of test-agent more clear. However, it isn't easy to make this change since it needs to modify a lot of tests in our current system. Gaia is pretty complicated and some of components such as system that wouldn't prefer to load by RequireJS.

I think it is possible to make RequireJS instead of our custom require call for better consistency in the future. :)
Thanks Evan,
But I only develop setup.js by moving and modifying code from the two files I mention previously.

I'm continuing this practice.
Attached file my setup.js — 
Hi Evan,

The first attachment is the "setup.js" I've made.
I implemented 'require' function by moving and modifying code from Ricky's reference[1]. I also replaced the function '_require' with Julien's remind[2].

Finally, I implement 'requireApp' function, but I faced a problem that said TestUrlResolver is not defined. I don't know whether I should implement it in setup.js as well or I should have brand new implementation of requireApp.

[1]https://github.com/mozilla-b2g/js-test-agent/blob/master/lib/test-agent/loader.js
[2]https://bugzilla.mozilla.org/show_bug.cgi?id=943163
Hi Veck,

It looks like we need to define TestUrlResolver[1] object in the window. Could you give it a try?

[1] https://github.com/mozilla-b2g/gaia/blob/master/dev_apps/test-agent/common/test/test_url_resolver.js
OK! Define TestUrlResolver in the window and got it run the test.
It seems it correctly work with this function, but still has a check error:

Firefox 27.0.0 (Mac OS X 10.9) ERROR
  Error: you must run tests using real domains on localhost
  at /Users/veck/setup.js:242

This error defined in TestUrlResolver, I don't have experience test on real domain but localhost.
What does this mean?
To run tests, we use separate URL for the different apps.
For example, for SMS, http://sms.gaiamobile.org:8080 is used.

Thanks to some preferences in the DEBUG profile, they all resolve to localhost. But the httpd server inside Firefox (in an extension) knows how to find the correct file using the host.

Hope this makes some sense :)
Hi all, the following is the gaia repo for this issue in my account:
https://github.com/fbukevin/gaia/tree/issue1027475

Whenever I have some progress, I will push it here to let everyone know my latest works

And thanks to Julien's help, I'll consider this point. :-)
Attached file First Test Result — 
I fixed the problem in need of real domain, with consideration that I can temporarily forsake using real domain to run test. I modified implementation of TestUrlResolver in setup.js. I updated code to GitHub.

The next problem is that "suite id not defined", so I added following snippet to my karma.conf.js as https://github.com/karma-runner/karma-mocha said:
    client: {
      mocha: {
        ui: 'tdd'
      }
    }

Finally, I load more two files to make test run.

This patch is the result I ran the test.
Nice job Veck!

I've seen your result on comment 21 and you make a good job to launch tests by karma.

In next step, we need to figure out why we get a bunch of failures on it. These "xxx is not defined" failures seem like a script loading problem that caused by executing test suites before script loading completed.

Another question is that it's possible to switch FF27 to nightly in karma? It may have some problems if we still stay in old version.
I don't know if that's relevant for your work, but with the recent httpd.js change (bug 1021481), we can (should !) use app:// instead of http:// to run tests.
Hi Veck,

Looks like the `requireApp` method could not work in your tree[1]. So there is a error like `VideoUtils is not defined` when we run the `apps/video/test/unit/video_utils_test.js` test.

I do some fixes, and the `video_utils_test.js` test works well now. You could check this branch[2] to see it.

Then let's fix the `requireApp` method.
Thanks.

[1] https://github.com/fbukevin/gaia/tree/issue1027475
[2] https://github.com/evanxd/gaia/tree/veck-bug-1027475
(In reply to Julien Wajsberg [:julienw] from comment #23)
> I don't know if that's relevant for your work, but with the recent httpd.js
> change (bug 1021481), we can (should !) use app:// instead of http:// to run
> tests.

Thanks for Julien's reminder.
We also need the `assert` object in the global `window`.
and the `sinon` object.
Thanks Evan!
I followed your opinion and the video_utils_test.js test works well, too.
Then I in turns wanted to test video.js. I loaded several files that either video.js or video_test.js would uses. Please check my update with [1].And I blocked on an error:

Firefox 27.0.0 (Ubuntu) ERROR
  TypeError: dom.player is null
  at /Users/veck/gaia/apps/video/js/video.js:37

To solve this problem, I found a bug report as [2].
I think the problem might occurred at that before I loaded gaia/apps/video/js/video.js, I didn't load any index.html to create dom.player in video.js. The only one index.html it should be loaded for this test is gaia/apps/video/index.html as I thought. But I still can't make this test passed.

[1] https://github.com/fbukevin/gaia/tree/issue1027475
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1013464
Hi Veck,

I think we should focus on implementing the `require` method, and we could build a karma plugin to do it.
After doing some investigation, looks like we could not build a karma plugin to achieve that. Karma doesn't support this kind of plugin[1] currently.

[1] http://karma-runner.github.io/0.10/dev/plugins.html
Hi Veck,

We could use the karma-chai-sinon[1] plugin to have assert and sinon in Karma. Once we use it, I think we don't need the `assert.js`[2] in the karma.conf.js file anymore.

[1] https://www.npmjs.org/package/karma-chai-sinon
[2] https://github.com/evanxd/gaia/blob/veck-bug-1027475/karma.conf.js#L19
And sorry, I change my word(Comment 30). We could just follow the karma-chai-sinon plugin do develop the plugin we want.
Hi Veck,

We already have the karma-test-agent plugin to instead of the `setup.js` file. Please add your require method there. So we don't need add `setup.js` file in the `karma.conf.js`[2], and the test also works well.

Thanks.

[1] https://www.npmjs.org/package/karma-test-agent
[2] https://github.com/evanxd/gaia/blob/veck-bug-1027475/karma.conf.js#L5-L8
Please provide your npm account here, and I will add you as the maintainer.
Status: NEW → ASSIGNED
Thank for your help Evan!
I am studying karma-chai-sinon!
np :)

BTW, how is the progress of `require` method?
I make the test-agent work and successfully use it to do test.
On the other hand, I keep studying how to make karma calls modules loaded on test-agent server.
I am thinking how to make karma call modules loaded by gaia test-agent server as they are different processes.
I'm trying to put all files that test of video would use in Karma file lists, and the remaining problem is, as I think, with the Error I mentioned in comment 28, video_test.js could not be trigger successfully.
Attached file video_test_result — 
After I modify apps/video/test/unit/video_test.js on line 55:
   loadBodyHTML('/index.html');
to be:
    loadBodyHTML('base/apps/video/index.html');

I got some successful test and still many are failed.
(In reply to Veck Hsiao from comment #41)
> Created attachment 8459311 [details]
> video_test_result
> 
> After I modify apps/video/test/unit/video_test.js on line 55:
>    loadBodyHTML('/index.html');
> to be:
>     loadBodyHTML('base/apps/video/index.html');
> 
> I got some successful test and still many are failed.

But we don't want to change this. You need to make it so that the browser's root directory ("/") will load files from the app's directory.

In current test agent, we use an iframe with URL "app://<app name>.gaiamobile.org", which uses the internal httpd.js (as an extension) which bounds this URL to the app directory.

Hope this makes sense !
Thanks Julien!

Do you mean that I should customize loadBodyHTML() function with root directory of gaia, just like my work of require and requireApp in setup.js?
It seems the problem of "TypeError: dom.player is null" is video.js always executes before video_test.js.

loadBodyHTML('/index.html'); is done in video_test.js makes the body of debug.html replaced with the body of index.html, so that video.js can getElementById correctly. But actually, video.js always executes before video_test.js such that the body of debug.html does not be replaced.
(In reply to Veck Hsiao from comment #44)
> It seems the problem of "TypeError: dom.player is null" is video.js always
> executes before video_test.js.
> 
> loadBodyHTML('/index.html'); is done in video_test.js makes the body of
> debug.html replaced with the body of index.html, so that video.js can
> getElementById correctly. But actually, video.js always executes before
> video_test.js such that the body of debug.html does not be replaced.

I dont know about video_test.js but in my app we usually have a "init" function that we can call after we change the HTML.

video.js does not seem to work this way, so maybe your work with karma does not work like we do with the test agent.


(In reply to Veck Hsiao from comment #43)
> Thanks Julien!
> 
> Do you mean that I should customize loadBodyHTML() function with root
> directory of gaia, just like my work of require and requireApp in setup.js?

No, what I mean is that when someone requests "/foo" it actually fetches the right directory. There are other locations were we don't use loadBodyHTML or require and use directly "/foo" (eg: media files), so this needs to work properly...
Hi Veck,

The video_test.js will load video.js inside itself. So we shouldn't put video.js [1] before video_test.js [2]. Execute video.js before will cause error at line: 37 since we haven't load index.html in video_test.js yet. Test-agent usually load source files (/video/js/*.js) from it's tests (/video/test/unit/*_test.js), and therefore I think you should remove all /video/js/*.js in karma.conf.


[1] https://github.com/RickyChien/gaia/blob/karmalize/karma.conf.js#L36
[2] https://github.com/RickyChien/gaia/blob/karmalize/karma.conf.js#L38
Karma has document how to serve but not to include source into browser in [1].

Using:

files: [
  ...,
  { pattern: 'apps/video/js/*.js', watched: false, included: false, served: true }
]

can access other files properly.

[1] http://karma-runner.github.io/0.8/config/files.html
Modify the pattern to '+(apps|shared)/**/*.*' is better.
Here is my patch [1] according to comment 46. As mentioned on comment 47, I think we should load essential scripts on "files" (see also [2]). The video_test.js will help us load other required scripts well.
 
There are an important information I want to tell you is our loader [3] cannot block Javascript execution. So If we do this in test-agent:

require('moduleA');
var module = new moduleA();

I cannot guarantee that moduleA will finish loading before we use it.

But if you use moduleA in mocha suite like this:

require('moduleA');

suite('test moduleA', function() {
  var module = new moduleA();
});

It works fine since test-agent has a magic in its loader. It will execute mocha.run() [3] after finishing all require() / requireApp() calls, but karma doesn't. We need to investigate where karma call mocha.run() and put mocha.run() after finishing all loading stuffs.

[1] https://github.com/RickyChien/gaia/blob/karmalize/karma.conf.js
[2] https://github.com/RickyChien/gaia/blob/karmalize/karma.conf.js#L17-L22
[3] https://github.com/RickyChien/gaia/blob/karmalize/dev_apps/test-agent/common/vendor/test-agent/test-agent.js#L3042-L3044
Thanks Ricky!

I followed your opinion, and I got dom.player problem solved.
The final problem is loadBodyHTML('/index.html'); could not successfully find /index.html.
I modified filename of loadBodyHTML() in shared/test/unit/load_body_html_helper.js to be the abs path, then all tests pass.
But just like Julien said, I should not change path of loadBodyHTML('/index.html'); in gaia/apps/video/test/unit/video_test.js. I'm surveying the better option.

Please check it with https://github.com/fbukevin/gaia/tree/issue1027475.
Attached image success.png — 
This is result of network in firebug of successful testing.
Attached image failed.png — 
While this is the result of network in firebug of failure.
The significant distinction is Request URL.
Again, why "/" does not point to your app directory?
I guess karma server host all served files on "gaia/". It doesn't have same rule as httpd.js, so loading resource from an absolute path will not work in here.
Then it will be very difficult to use it... some app _code_ assume this too.
Attached image video_test_success.png — 
I made all *_test.js in apps/video/test/unit succeed.
And then I try to test apps/bluetooth.
Because the karma.config.js and setup.js for these tests have a little bit of difference with video, I create bluetooth.config.js and bluetooth_setup.js for bluetooth test.
The difference of setup.js is that all paths of requireApp in apps/bluetooth/test/unit/*_test.js are starting from "bluetooth" while in apps/video/test/unit/*_test.js are starting from "/video". Please check [1].

There 37 success in 70 tests.
Most of errors show that "this.sinon is not defined".

[1]https://github.com/fbukevin/gaia/tree/issue1027475
Hi Veck,

Nice works for the video tests.

But we should have NO any setup.js(video_karma_setup.js, bluetooth_setup.js) here, we should make all changes in the karma-test-agent[1] plugin.

We should let requireApp work in the tests. So we don't need `shared/test/unit/mocks/mocks_helper.js` in the `karma.conf.js` file[2], and keeping one karma config file(karma.conf.js) is the way we are going to.

[1] https://www.npmjs.org/package/karma-test-agent
[2] https://github.com/fbukevin/gaia/blob/issue1027475/karma.conf.js#L20
Thanks to remind me, Evan!

I put both video_karma_setup.js and bluetooth_setup.js to karma-test-agent repo.
I use them in my gaia repo for developing.

I separate video.conf.js and bluetooth.conf.js for testing of video and bluetooth respectively, I'll put them together after all test going well. 

"shared/test/unit/mocks/mocks_helper.js" is used in video_test.js and it could not be found is we only served it instead of included it.
Test-agent loads every app/test/unit/setup.js before runnin tests (if setup.js not found, we do nothing). If someone needs mocks_helper.js to mock their tests, they will include it in setup.js by itself. With such a manner to run their tests properly.

So, I think we should find and load every "app/test/unit/setup.js" if it does exists. Putting this part of job in karma-test-agent and make sure we can load our libs dynamically before running tests.
Hi, Evan!
I remove the duplicated karma.config.js and still keep video.js work, while mocks_helper.js is needed to be loaded. I adapt Ricky's advise making a loader (testing file name is bluetooth_loader.js) to detect if apps/*/test/unit/setup.js exists or not. The problem is that When I require('utils'); to use "utils.getFile()" for "var file" and use "file.exists()", utils always not defined.

I think the problem might caused by our self-defined require(). Or do you have better solution?
Thanks!
Attached file all_test.txt — 
I created *.conf.js for each apps and dev_apps that have test/unit/*_test.js files. And the karma testing result is listed in this attachment.
Hi all, in this week, I'm going to solve problem as following:
1. this.sinon is not defined. (error occurs, ex. apps/bluetooth)
2. requrieElement, requireLib, requireCommon,.. is not defined. (error occurs, ex. apps/calendar)
3. loadBodyHTML could not find path. (error occurs, ex. apps/settings)
To fix apps/calendar problem, I create this branch: https://github.com/fbukevin/gaia/tree/veck_calendar
I load apps/calendar/test/unit/setup.js and apps/calendar/test/unit/laoder.js so that requireElement, requrieLib and requireCommon problems solved.

To start test, you should enter gaia/node-modules/karma-test-agent/lib/apps_config. Then type in "karma start calendar.conf.js".

But new error is "navigator.mozL10n is undefined". I found that occurs at "gaia/apps/calendar/test/unit/setup.js:305:11". But the line is as following:

   var state = navigator.mozL10n.readyState;

and mozL10n is loaded at line 296:
   requireApp('calendar/shared/js/l10n.js');

The problem is that the directory calendar/shared does not exist?
Yes, I think so.
So we could move the `l10n.js` file[1] into the `calendar/shared` folder.
[1] https://github.com/mozilla-b2g/gaia/blob/master/shared/js/l10n.js
Move the file at the runtime of running tests.
Depends on: 1073383
Depends on: 1073385
Gaia Testing Result via karma-test-agent:

https://hackpad.com/Gaia-Testing-Result-via-karma-test-agent-X2euT4Qin9C

The future works will mainly focus on resolving all the errors.
Blocks: 1108947
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: