openbci-archive / OpenBCI_NodeJS

Node.js SDK for the all OpenBCI Biosensor Boards
https://www.npmjs.com/package/openbci
137 stars 52 forks source link

Tests Rely on Undocumented Internal State #90

Open baffo32 opened 7 years ago

baffo32 commented 7 years ago

Some of the tests manually set internal state variables, such as .connected, in order to test that functions change this state. This makes for a lot of work to update all the tests if the internal state management changes. The code would be more maintainable if tests used public API functions to alter state when this is reasonable to do.

Approach I plan:

andrewjaykeller commented 7 years ago

Love it. I never thought about how much that would help maintainability.

I was doing that because i really wanted to isolate the functionality of the function under test.

andrewjaykeller commented 7 years ago

I apologize for the convoluted tests! I did not intend to make the tests non-condusive to productivity. This is simply not acceptable!

andrewjaykeller commented 7 years ago

This has totally changed the way I have been writing my tests for my main project (the one that uses this library) and holy smokes totally makes more sense to use public API method calls throughout and not muck with the internals. GOOD CALL Karl!