search

philip1986 / pimatic-led-light

A template for creating plugins
http://pimatic.org/
GNU General Public License v2.0
7 stars 13 forks source link

Suggested fixes for issues #18, #19, and #24 #25

Closed mwittig closed 8 years ago

mwittig commented 9 years ago

See commit log for details

philip1986 commented 9 years ago

I think we reach a degree of complexity were we need tests to ensure the stability of our code. I already thought about an approach for tests like this, which takes also in account that you like to split the plugin by devices . So I will try to add some basic tests.

philip1986 commented 9 years ago

https://github.com/philip1986/pimatic-led-light/pull/25/files#diff-fc25f60a59d04b30ef5515738aff8bc6R55 this line is also broken @power is either 'on' or 'off', so the condition is always true.

mwittig commented 9 years ago

Agree on testing :) Note, however, the power attribute should be true/false to get properly written to the database as far as I know. The dummy and blinkstick device implementations also use true/false btw.

philip1986 commented 9 years ago

Yeh we need to refactor this on/off implementation, its also quite confusing I just realized it during writing the tests.
So this PR is OK.

philip1986 commented 9 years ago

@mwittig checkout https://github.com/philip1986/pimatic-led-light/pull/26. This is what I have so far in terms of testing. please let me know what you thing about this approach.

Some tests are skipped because of broken code. This the includes also a test for setColor on milight, which should be fixed by this PR.

mwittig commented 9 years ago

Hi @philip1986 I had a frist look at your test effort which I really like. what do I need to checkout to run the tests? Can I simply chckou the "feature/add_tests" branch?

philip1986 commented 9 years ago

@mwittig ok than I will go on whit this tests. Yes just checkout the branch and run npm install.

mwittig commented 8 years ago

@philip1986 Added suggested fix for issue #19