Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Travis should run the js tests #801

Merged
merged 15 commits into from Jul 23, 2013
Merged

Travis should run the js tests #801

merged 15 commits into from Jul 23, 2013

Conversation

domoritz
Copy link
Contributor

We have some JavaScript tests we should run these along with the python ones

@ghost ghost assigned johnmartin Jul 16, 2013
@johnmartin
Copy link
Contributor

We should also try and have more up to date Mocha tests.

@tobes
Copy link
Contributor Author

tobes commented Jul 16, 2013

how do we run them? I could add them to travis

can you add a test that fails for testing purposes

@johnmartin
Copy link
Contributor

@tobes

@johnmartin
Copy link
Contributor

Also, I was just looking into adding the tests to travis... but I'm not sure how to do that. If you know, I'd be happy to re-assign...

@tobes
Copy link
Contributor Author

tobes commented Jul 16, 2013

we need to run them outside the browser

can we run them via nodejs at all? or some other command line thing?

@johnmartin
Copy link
Contributor

@tobes Yes, we should be able to run them via node. Lemme take a look and then I'll get back to you...

@domoritz
Copy link
Contributor

Have a look at zombie http://zombie.labnotes.org.

@johnmartin
Copy link
Contributor

@tobes

OK, I've got the front end tests running via the command line via mocha-phantomjs [1], it's essentially a mocha wrapper for phantomjs (which is a headerless node.js webkit browser). Here's what you'll need in order to get it to work:

  1. git checkout 801-travis-run-mocha-tests
  2. Install deps: npm install -g mocha-phantomjs phantomjs
  3. Run the test: mocha-phantomjs -s webSecurityEnabled=false ckan/public/base/test/index.html

Now the only issue is that you do require a CKAN instance to be running at http://localhost:5000 in order to complete a few of the tests (the test logic requires a call to the snippet API call to test against it's dom... but if we can't get this to work I think we just rewrite those tests)

Also, as it stands that branch should error at: ckan.module.RelatedItemModule() "before all" hook but that's a known issue and it'll be fixed in #1082 (you should be able to cherry-pick 05096e9 to see it fully passing all tests)

@domoritz Thanks for the tip... as you can see, I ended up going for something similar

[1] https://github.com/metaskills/mocha-phantomjs

@domoritz
Copy link
Contributor

@johnmartin I made this a pr so that we can see what's happening on travis. Hope that's okay.

@tobes
Copy link
Contributor Author

tobes commented Jul 17, 2013

I was doing stuff on this but will stop as you are too

I get 3 fails (I didn't start ckan) so this seems like easy enough to get working

@domoritz
Copy link
Contributor

@tobes Are you talking to me? I also stopped doing stuff.

@tobes
Copy link
Contributor Author

tobes commented Jul 17, 2013

I talk to anyone who will listen

@johnmartin
Copy link
Contributor

@tobes @domoritz

OK, well... I'm happy with the mocha tests running from command line now. I'm not sure what the next steps on getting travis to run them though...

@tobes
Copy link
Contributor Author

tobes commented Jul 18, 2013

https://travis-ci.org/okfn/ckan/jobs/9219402 looks like travis ran the tests

what we need is a failing test to check travis notices

We also want to try to reduce the amount of noise

  1. do all the install stuff before any tests get run - also look at that npm json issue on travis (is it important?)
  2. running the js tests before the nose ones might keep the devs happy as then there is minimal change to the travis output (we probably need to stop the ckan instance that we start up too)

@johnmartin
Copy link
Contributor

According to that travis job: yes all the node deps have been installed correctly. The three warnings are minor ones (which we don't need to worry about). Also, yes npm it's very noisey when installing things with a load of deps... however there are only ways to make npm more verbose not less verbose.

@tobes
Copy link
Contributor Author

tobes commented Jul 18, 2013

cool just do the installs before our ini file output, nosetests etc so we can ignore them we could do something like npm install xxx > /dev/null but it is probably best to keep the info in case we have actual issues.

@johnmartin
Copy link
Contributor

@tobes Perfect. This is good for review. Removing WIP.

@tobes
Copy link
Contributor Author

tobes commented Jul 18, 2013

I still want the nosetests run last as that is least disruptive for the devs as then going to the end of the tests will show the same output that we are used to also the majority of peoples test errors will be python based as that is where the bulk of commits occur

we will need to kill our test server before we run those tests though or we will have db locking issues/ port in use

@johnmartin
Copy link
Contributor

@tobes OK, I can move the mocha tests to a better place... however I don't know where to start with killing the paster serve...

@tobes
Copy link
Contributor Author

tobes commented Jul 18, 2013

kill %1 should kill the server when all your tests are done

did you add a test that fails just to test travis will pick it up

@johnmartin
Copy link
Contributor

@tobes Thanks. dd46f25 adds a failing test for travis.

@tobes
Copy link
Contributor Author

tobes commented Jul 23, 2013

@johnmartin ok the tests are running now. I'm working on getting travis to know the error happened. Hopefully my last commit will do this.

@tobes tobes merged commit 89dd6ec into master Jul 23, 2013
@tobes tobes deleted the 801-travis-run-mocha-tests branch July 23, 2013 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants