Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3560)

Issue 7201049: PhantomJS Testing support

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 9 months ago by bcsaller
Modified:
11 years, 9 months ago
Reviewers:
frankban, mp+144629, gary.poster
Visibility:
Public.

Description

PhantomJS Testing support Change test server to use phantomjs. To get this branch working you'll need sudo npm install -g mocha-phantomjs After that make test-debug and test-prod should work with CLI exit codes and .lbox.check should run the tests as well. https://code.launchpad.net/~bcsaller/juju-gui/phantom-testing/+merge/144629 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 9
Unified diffs Side-by-side diffs Delta from patch set Stats (+62 lines, -73 lines) Patch
M .lbox.check View 1 chunk +1 line, -1 line 0 comments Download
M Makefile View 1 chunk +1 line, -1 line 1 comment Download
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M app/app.js View 5 chunks +11 lines, -23 lines 2 comments Download
M app/assets/javascripts/d3-components.js View 3 chunks +2 lines, -3 lines 2 comments Download
M app/models/models.js View 2 chunks +2 lines, -5 lines 2 comments Download
M app/store/env.js View 8 chunks +12 lines, -12 lines 2 comments Download
M app/views/charm.js View 3 chunks +3 lines, -3 lines 0 comments Download
M app/views/environment.js View 1 chunk +0 lines, -1 line 0 comments Download
M app/views/unit.js View 2 chunks +2 lines, -2 lines 0 comments Download
M app/views/utils.js View 4 chunks +7 lines, -7 lines 0 comments Download
M test-server.sh View 1 chunk +4 lines, -2 lines 0 comments Download
M test/index.html View 2 chunks +11 lines, -9 lines 0 comments Download
M test/test_env.js View 1 chunk +2 lines, -2 lines 0 comments Download
M test/utils.js View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 6
bcsaller
Please take a look.
11 years, 9 months ago (2013-01-24 04:18:52 UTC) #1
gary.poster
Trivial comments. Thank you for doing this! It looks good. I will give it a ...
11 years, 9 months ago (2013-01-24 05:06:06 UTC) #2
frankban
Thanks for this branch Ben, it's cool to have a meaningful exit code from our ...
11 years, 9 months ago (2013-01-24 12:18:35 UTC) #3
gary.poster
Hey. I duped Francesco's failure and dug into it a bit for exercise :-) . ...
11 years, 9 months ago (2013-01-24 13:04:16 UTC) #4
gary.poster
To be clear, my vote is "land with changes" Thanks
11 years, 9 months ago (2013-01-24 13:06:59 UTC) #5
bcsaller
11 years, 9 months ago (2013-01-24 13:40:21 UTC) #6
On 2013/01/24 13:04:16, gary.poster wrote:
> Hey.  I duped Francesco's failure and dug into it a bit for exercise :-) .  It
> turns out that the problem for us is simply that mocha-phantomjs can't find
> phantomjs.  This is the pertinent code in /usr/bin/mocha-phantomjs:
> 
>   85 var phantomjs;
>   86 for (var i=0; i < module.paths.length; i++) {
>   87   var bin = path.join(module.paths[i], '.bin/phantomjs');
>   88   if (process.platform === 'win32') {
>   89     bin += '.cmd';
>   90   }
>   91   if (exists(bin)) {
>   92     phantomjs = spawn(bin, spawnArgs);
>   93     break;
>   94   }
>   95 }
>   96 if (phantomjs === undefined) { phantomjs = spawn('phantomjs', spawnArgs);
}
> 
> module.paths for me was this:
> 
> [ '/usr/lib/node_modules/mocha-phantomjs/bin/node_modules',
>   '/usr/lib/node_modules/mocha-phantomjs/node_modules',
>   '/usr/lib/node_modules',
>   '/usr/node_modules',
>   '/node_modules' ]
> 
> .bin/phantomjs does not exist in any of those locations, and then the last
check
> for a global phantomjs fails.
> 
> I found that if I simply installed a global phantomjs (`sudo npm install -g
> phantomjs`) the tests ran great.
> 

Thanks.


> Unless someone has objections to all of us also installing phantomjs globally,
> these are my additional suggestions beyond the previous trivial comments.
> 
>  * In the "Developer Install" section of the HACKING document, record the
> instructions to install both mocha-phantomjs and phantomjs globally.  (Note
that
> orange squad is coming on soon and will need that HACKING document to actually
> work!)
> 
>  * Make a separate Makefile target for simply starting the test server.  For
> now, I'd like for us to still be able to run the tests occasionally in
browsers,
> and in the future, this is the kind of thing we will need for CI test
services.

Agreed. I got enthusiastic once I started to get it working. I think we can
default to prod for the integrated test and debug for browser (as you'll most
likely be using the debugger at this point).
 
> 
> Thanks
> 
> Gary
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b