[extensions] Better document. Add unit tests. (#1248) #1442
Conversation
tests/src/rules/extensions.js
Outdated
|
|
||
| const ruleTester = new RuleTester() | ||
|
|
||
| describe('extensions buildProperties', function() { |
There was a problem hiding this comment.
i'm confused what the purpose of these tests are. can the tests be exercised with ruleTester test cases, rather than hardcoding to things like __test__?
There was a problem hiding this comment.
Sorry, I deleted my previous comment by accident. I did it like that, because buildProperties is private function and needs to be tested.
So I guess it's a no go for this PR?
There was a problem hiding this comment.
in general, private things shouldn't be tested - in this plugin's case, everything should be tested via the RuleTester.
There was a problem hiding this comment.
I'm not sure why you're saying "private things shouldn't be tested". I see no reason not to, and plenty of reasons to do.
There was a problem hiding this comment.
One benefit of not testing internal functions is that you can refactor internals as much as you want.
There was a problem hiding this comment.
But when you refactor internals and break something, you wish you had tests to catch that early.
There was a problem hiding this comment.
The tests should be on the public pieces, and should transparently exercise all the private pieces.
Any behavior of your private code that is not observable via your public code should not be tested because it doesn't matter if it works or not
I was curious of how this rule worked, so I went the easiest way and wrote a bunch of unit tests for the buildProperties (a method that returns the options from the config for the extensions rule) method.