Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests Rely on Undocumented Internal State #90

Open
3 tasks
baffo32 opened this issue Oct 16, 2016 · 3 comments
Open
3 tasks

Tests Rely on Undocumented Internal State #90

baffo32 opened this issue Oct 16, 2016 · 3 comments

Comments

@baffo32
Copy link
Contributor

baffo32 commented Oct 16, 2016

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
@andrewjaykeller
Copy link

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
Copy link

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

@andrewjaykeller
Copy link

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants