search

mgenev / how-to-sane

A demonstration of how to use the SANE stack
http://howtosane.ninja
MIT License
163 stars 22 forks source link

Add test framework and example test #2

Closed IanVS closed 9 years ago

IanVS commented 9 years ago

Tasks before merge-ready:

Added:

To execute tests, npm test inside server directory.

So far, only one test added to show that it works.

Globegitter commented 9 years ago

@IanVS Another great contribution. Instead of grunt though, do you think you can just make it in such a way that tests are run via npm test? Similar to how sane does it? https://github.com/artificialio/sane/blob/master/package.json#L8 and https://github.com/artificialio/sane/blob/master/tests/runner.js

Edit: Now that I am thinking about it, maybe we can even make a simple npm module out of this, similar to how machinepacks do it: https://github.com/mikermcneil/machinepack-npm/blob/master/package.json#L6

Reason for this being, that I think grunt is generally a quite heavy dependency to have and it is not too hard to just do something more lightweight yourself :)

IanVS commented 9 years ago

Sure I can try doing it that way. Reason I used grunt was partly because Sails uses it natively for its task running, partly because I wanted to get a little familiar with how to set up grunt tasks, and the biggest reason...that's how it was done in the tutorial I was following. :)

I'll try refactoring tonight.

Globegitter commented 9 years ago

Yeah grunt is the old way :P Sails does support gulp now as well I believe, which is better than grunt but yeah I think it still is a bit too much overhead just for running tests :)

But that is good stuff.

mgenev commented 9 years ago

Hey, @IanVS this is awesome! I will review when i've had some sleep. I'm not sure if I showed you my list of suggested features for SANE, but the test framework was towards the top, so this is pretty perfect

IanVS commented 9 years ago

Yeah I wanted to start playing around with adding functionality and tweaks, and wanted to make sure I didn't break any of your code. I also plan to give BDD a shot, even though I really have only the faintest clue what it is and how to do it. Learning as I go here.

I'll be updating the tests tonight to use straight npm as @Globegitter suggested. Looks very straight forward to do. So don't review just yet.

I'm also investigating git hooks to prevent tabs from being committed/pushed, as @kriswill called me out on.

IanVS commented 9 years ago

By the way, if I know there's additional work to be done on a PR, is it best practice to close the PR for the time being, until issues are addressed and I think it's ready to merge?

kriswill commented 9 years ago

@IanVS ember-cli has .jshintrc tests in their standard suite. see https://github.com/ember-cli/ember-cli/blob/master/tests/unit/jshint-test.js as a side note I personally use jshttp javascript style -- https://jshttp.github.io

Globegitter commented 9 years ago

There are different conventions per project, but usually not closing the PR. You could either just mention Not ready to merge at the top if you know from the start it is not ready, and/or a checklist with a list of things that already has been done and things that are still missing. Then it really just has to pass at least one of the maintainers code review and tests if any are present.

mgenev commented 9 years ago

@IanVS for BDD check out this https://github.com/LateRoomsGroup/moonraker I use it at work and like it a lot

IanVS commented 9 years ago

@mgenev, moonraker looks cool, but I guess my goal is to stick fairly closely to the syntax used in ember-cli and sane (although they're much more sophisticated than this is so far).

@Globegitter I'm sorry but I have no idea how machinepacks work or how to make this an npm module. Maybe that's something we can do later? I'd like to try to do more work on making tests and improving auth.

kriswill commented 9 years ago

I think it's a good start!

I think what @Globegitter would like to see, is how we can leverage the work in this project to integrate with sane-cli's addon system (which is still very alpha).

Eventually we would like to expose rich enough interfaces that nearly every piece of the Sane stack is implemented using the addon system. This would give us immense flexibility to create new features, and allow the community to contribute alternative capabilities into the stack, like new deployment targets, configuration systems, alternate testing frameworks, new code generator artifacts, etc. The future is bright!

mgenev commented 9 years ago

@IanVS with that cucumber/yadda, since it ends up in plain English, I set it up so our QA staff that doesn't code, writes out the behaviors of the product specs we get and I don't have to worry about that part :D

Globegitter commented 9 years ago

@IanVS yes sorry I should have been a bit more specific, this where more thoughts for the future. I have now learned to write down ideas immediately somewhere :) What you have right now is a really good and I would say LGTM (let's get this merged) if @mgenev does not have any objections.

And yes @kriswill is explaining the core idea in a bit more detail. But again nothing to worry for now.

Globegitter commented 9 years ago

And regards BDD and moonraker haven't really done that myself yet, but I think these are things we can tackle in future PRs as well. It is good to have this basis for now.