search

stellar-deprecated / stellar-client

INACTIVE. Browser based client for stellard. This repository is inactive. It points to the stellard network, which is being replaced by stellar-core. Please refer to the replacement repository, interstellar-client, which points to the stellar-core network.
Other
307 stars 74 forks source link

first karma test passing #1151

Closed gterzian closed 9 years ago

gterzian commented 9 years ago

Work in progress - setting up a suite of unit and integration tests for the controllers/services/directives parts of the Stellar client.

Please note I had to include some quick and dirty mocks to get the code to run in a Karma environment. Pls let me know if I missed anything on that side.

Also please let me know if you have any preferences regarding folder structure and naming.

First test passing :)

nullstyle commented 9 years ago

Hi @gterzian. Thanks for the contribution!

Just a couple of notes... The intent for the gulp test task is to be the trigger for a comprehensive test run. I would recommend that you perhaps rename the current gulp test (which runs the protractor tests) into something like test:e2e, add a new task for the karma tests (perhaps test:unit) and then trigger them one after the other inside of a parent test task using runSequence. In that way, a dev can choose to run a subset of tests as appropriate, but TravisCI will still run the full suite without the need for additional configuration. Does that make sense?

In regards to naming, I would prefer we don't specifically mention karma in our folder structures... karma is just the driver we have to run the tests, but I would prefer the directory structure expresses the role those files take within our testing strategy (i.e. e2e for end to end tests, and unit for unit tests).

gterzian commented 9 years ago

thanks @nullstyle that all makes sense. I'm not so familiar with gulp so I'll first look more into it and get those tests to run as part of the main gulp test.

Thanks for the suggestions on the folder structure/naming

gterzian commented 9 years ago

@nullstyle Allright looks like the switch to run it all as part of gulp test is working fine.

I'll write some actual tests next...

gterzian commented 9 years ago

@nullstyle Hi Scott how do you want to approach this going forward? The basic 'infrastructure' is now in place and there's one controller covered, one service mocked, just for starters.

I'de like to write more tests and I don't know how much time I'll have this week so perhaps we want to first merge this and later I'll refork master?

Also I started making small changes to the controllers to aid in testing and then figured this would just make it harder to merge and we would have to re-QA everything as well. So I'm not touching any 'app' code for now.

On the other hand when something is hard to test it's maybe a good opportunity to rethink how it was done.

Please let me know your thoughts.

nullstyle commented 9 years ago

Hi @gterzian!

It seems totally fine to merge this in now. I'll do a quick once-over again and merge if everything looks kosher. I lean far towards the "Keep the pull request small" camp, so merging the current work you have in place is preferable.

gterzian commented 9 years ago

@nullstyle ok great, thanks!

nullstyle commented 9 years ago

Oh, I forgot to mention in my last message @gterzian: Thanks! We really appreciate the contribution.

gterzian commented 9 years ago

@nullstyle you're welcome

I'll add more tests soon, I enjoy seeing how other people structure their project and code.

And Stellar is doing something important so I'm happy to contribute in some way :smile: