Turn test and build scripts into Makefile targets

[emanchado]

Turn run-tests.sh, run-selenium-tests.sh and build.sh into Makefile targets. The scripts are still there for compatibility, but now they just call "make ".

This is a proposed fix for #227.

As an extra, the "clean" target has improved.

[Rangak]

Hi Esteban, This is great ! Thanks. Looks like it will take some more work before getting it merged (Travis CI is failing) and I also want to test it on a clean copy to check it out. Thanks tor getting this going.

[patrickod]

There's a dependency issue with the packages.json as is. The karma-phantomjs-launcher. Specifically the version string it defines for the phantomjs package here https://github.com/karma-runner/karma-phantomjs-launcher/blob/master/package.json#L20 does not work.

I observe the following output when running make test locally using node v0.10.31 and npm version 2.0.0

./node_modules/handlebars/bin/handlebars -f /tmp/tmp.gSt63Z0m5u_templates.js app/templates/*.handlebars  
if ! diff --brief /tmp/tmp.gSt63Z0m5u_templates.js app/js/templates.js; then \
    echo 'Error: templates.js out-of-date. Run `make app/js/templates.js`' 2>&1; \
    exit 1; \
fi
npm install
npm WARN package.json karma-chrome-launcher@0.1.4 No README data
npm ERR! Linux 3.16.2-1-ARCH
npm ERR! argv "/usr/bin/node" "/usr/bin/npm" "install"
npm ERR! node v0.10.31
npm ERR! npm  v2.0.0
npm ERR! code ETARGET

npm ERR! notarget No compatible version found: phantomjs@'>=1.9.0 <1.10.0'
npm ERR! notarget Valid install targets:
npm ERR! notarget ["0.0.1","0.0.2","0.0.3","0.0.4","0.0.5","0.0.6","0.0.7","0.0.8","0.0.9","0.1.0","0.1.1","0.2.0","0.2.1","0.2.2","0.2.3","0.2.4","0.2.5","0.2.6","1.8.0-1","1.8.1-1","1.8.1-2","1.8.1-3","1.8.2-0","1.8.2-1","1.8.2-2","1.9.0-0","1.9.0-1","1.9.0-2","1.9.0-3","1.9.0-4","1.9.0-5","1.9.0-6","1.9.1-0","1.9.1-2","1.9.1-3","1.9.1-4","1.9.1-5","1.9.1-6","1.9.1-7","1.9.1-8","1.9.1-9","1.9.2-0","1.9.2-1","1.9.2-2","1.9.2-3","1.9.2-4","1.9.2-5","1.9.2-6","1.9.6-0","1.9.7-1","1.9.7-3","1.9.7-4","1.9.7-5","1.9.7-6","1.9.7-7","1.9.7-8","1.9.7-9","1.9.7-10","1.9.7-11","1.9.7-12","1.9.7-13","1.9.7-14","1.9.7-15","1.8.2-3"]
npm ERR! notarget 
npm ERR! notarget This is most likely not a problem with npm itself.
npm ERR! notarget In most cases you or one of your dependencies are requesting
npm ERR! notarget a package version that doesn't exist.
Makefile:49: recipe for target 'deps' failed
make: *** [deps] Error 1

[patrickod]

Turns out this is actually an issue w/ npm@v2.0.0 which is now a bit more specific when it comes to version strings karma-runner/karma-phantomjs-launcher#42. Specifically the phantomjs version numbers being nonstandard causes issues

[patrickod]

Forked the karma-phantomjs-launcher to have a working dependency string. The following output is now observed from make test

./node_modules/handlebars/bin/handlebars -f /tmp/tmp.eRRni5mjxS_templates.js app/templates/*.handlebars  
if ! diff --brief /tmp/tmp.eRRni5mjxS_templates.js app/js/templates.js; then \
    echo 'Error: templates.js out-of-date. Run `make app/js/templates.js`' 2>&1; \
    exit 1; \
fi
npm install
npm WARN package.json karma-chrome-launcher@0.1.4 No README data
npm WARN package.json karma-ie-launcher@0.1.5 No README data
pip install  -qr requirements.txt
which: no nodejs in (/home/patrick/.rbenv/shims:./node_modules/.bin:/home/patrick/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/sbin:/usr/X11/bin:/home/patrick/arm-cs-tools/bin:/usr/local/go/bin:/home/patrick/src/android-sdk-macosx/tools:/home/patrick/src/android-sdk-macosx/platform-tools:)
/usr/bin/env python2.7 -m unittest discover -s test/ -p '*_test.py'
.....................................
----------------------------------------------------------------------
Ran 37 tests in 0.032s

OK
node -e "require('grunt').tasks(['test']);"
Running "karma:unit" (karma) task
INFO [karma]: Karma v0.10.10 server started at http://localhost:9876/
INFO [launcher]: Starting browser PhantomJS
INFO [PhantomJS 1.9.7 (Linux)]: Connected on socket idzl1imC01T5wdKzTqsv
WARN [web-server]: 404: /cgi-bin/routerapi/dashboard
WARN [web-server]: 404: /cgi-bin/routerapi/setup_state
WARN [web-server]: 404: /cgi-bin/routerapi/settings
WARN [web-server]: 404: /cgi-bin/routerapi/bytecount
WARN [web-server]: 404: /cgi-bin/routerapi/bytecount
PhantomJS 1.9.7 (Linux): Executed 1 of 22 SUCCESS (0 secs / 0.02 secs)
PhantomJS 1.9.7 (Linux): Executed 2 of 22 SUCCESS (0 secs / 0.023 secs)
PhantomJS 1.9.7 (Linux): Executed 3 of 22 SUCCESS (0 secs / 0.026 secs)
PhantomJS 1.9.7 (Linux): Executed 4 of 22 SUCCESS (0 secs / 0.028 secs)
PhantomJS 1.9.7 (Linux): Executed 5 of 22 SUCCESS (0 secs / 0.032 secs)
PhantomJS 1.9.7 (Linux): Executed 6 of 22 SUCCESS (0 secs / 0.035 secs)
PhantomJS 1.9.7 (Linux): Executed 7 of 22 SUCCESS (0 secs / 0.036 secs)
PhantomJS 1.9.7 (Linux): Executed 8 of 22 SUCCESS (0 secs / 0.039 secs)
PhantomJS 1.9.7 (Linux): Executed 9 of 22 SUCCESS (0 secs / 0.041 secs)
PhantomJS 1.9.7 (Linux): Executed 10 of 22 SUCCESS (0 secs / 0.043 secs)
PhantomJS 1.9.7 (Linux): Executed 11 of 22 SUCCESS (0 secs / 0.047 secs)
PhantomJS 1.9.7 (Linux): Executed 12 of 22 SUCCESS (0 secs / 0.049 secs)
PhantomJS 1.9.7 (Linux): Executed 13 of 22 SUCCESS (0 secs / 0.051 secs)
PhantomJS 1.9.7 (Linux): Executed 14 of 22 SUCCESS (0 secs / 0.053 secs)
PhantomJS 1.9.7 (Linux): Executed 15 of 22 SUCCESS (0 secs / 0.054 secs)
PhantomJS 1.9.7 (Linux): Executed 16 of 22 SUCCESS (0 secs / 0.055 secs)
PhantomJS 1.9.7 (Linux): Executed 17 of 22 SUCCESS (0 secs / 0.059 secs)
PhantomJS 1.9.7 (Linux): Executed 18 of 22 SUCCESS (0 secs / 0.059 secs)
PhantomJS 1.9.7 (Linux): Executed 19 of 22 SUCCESS (0 secs / 0.059 secs)
PhantomJS 1.9.7 (Linux): Executed 20 of 22 SUCCESS (0 secs / 0.059 secs)
PhantomJS 1.9.7 (Linux): Executed 21 of 22 SUCCESS (0 secs / 0.059 secs)
PhantomJS 1.9.7 (Linux): Executed 22 of 22 SUCCESS (0 secs / 0.059 secs)
WARN [web-server]: 404: /cgi-bin/routerapi/change_password
PhantomJS 1.9.7 (Linux): Executed 22 of 22 SUCCESS (0.363 secs / 0.059 secs)

Done, without errors.

The test suite seems to pass though it does note quite a few 404s. @emanchado have you observed the same 404s listed in your own output while testing?

[emanchado]

@patrickod I do get the 404s using node 0.10.21 and npm 1.3.11, I just never notice them. Or maybe I thought they were normal.

In any case, I haven't touched anything related to those tests so I would suggest to merge this patch if it's otherwise fine, and file a separate ticket to investigate if the 404s are a problem.

[patrickod]

Ah apologies, forgot to comment last night that the 404 issues are common to the old test scripts. They're not as a result of this PR.

Outside of the dependency issue w/ karma-phantomjs-launcher I think this can and should be merged.

The only caveat are the dependency issues w/ latest npm. I've forked the karma-phantomjs-launcher package to resolve these. Maybe we can change the karma-phantomjs-launcher version to point to git://github.com/patrickod/karma-phantomjs-launcher#v0.1.5 to fix this ?

[emanchado]

I don't really have strong opinions either way (but then again, I'm just a random contributor, not part of the core team or anything). Personally I think it would be kind of nicer for karma-phantomjs-launcher to fix the issue on their side but if they take much longer, working around it sounds better.

[patrickod]

There's an issue opened on Github for this which I linked above. The project hasn't been touched since April so it's possibly going to be a while.

I'm on this ticket so once I closes I'll happily issue a PR to revert the workaround.

On Wed, Sep 17, 2014 at 4:58 AM, Esteban Manchado Velázquez
notifications@github.com wrote:

I don't really have strong opinions either way (but then again, I'm just a random contributor, not part of the core team or anything). Personally I think it would be kind of nicer for karma-phantomjs-launcher to fix the issue on their side but if they take much longer, working around it sounds better.

Reply to this email directly or view it on GitHub:
#245 (comment)

[coffeemakr]

This issue is caused by a little bug in your Makefile. The --user option is appended IF we are in a virtualenv instead when we're not. Just switch the arguments of the if statement and the tests run like a charm.

diff --git a/Makefile b/Makefile
index 467a296..aabaece 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,5 @@
 NODEJS=$(if $(shell which nodejs),nodejs,node)
-PIP_USER_SWITCH=$(if $(VIRTUAL_ENV),--user,)
+PIP_USER_SWITCH=$(if $(VIRTUAL_ENV),,--user)

 TEMPLATES_JS=app/js/templates.js
 HANDLEBARS_FILES=app/templates/*.handlebars  # Used to generate templates.js

PS: I've reproduced the error, applied the fix and it worked.

[Rangak]

There are several items here that are reasons for caution, or will break things. Defer merge of these makefile related changes until reorg plan to make an openwrt package from this project ( Issue #168 ) takes shape.