openbci-archive/OpenBCI_NodeJS

Tests Rely on Undocumented Internal State

Opened this issue · 3 comments

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:

  • review all tests for patterns such as .member = value or ._member
    • change unneccessary ones to use public API functions in case the code assumes more state than the test does
    • for cases where this seems necessary, ensure the use of internal state matches the assumptions the internal code makes, and add a todo item or issue to mention the use

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.

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

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!