some tests around the NpmList.maybePushNewCoordinate() logic. #188
Conversation
| }); | ||
| }); | ||
|
|
||
| describe('NpmList.maybePushNewCoordinate', async () => { |
There was a problem hiding this comment.
I don't have a gigantic problem with this, but this is a private method. Generally I'd want to test the publics, and not tie directly to privates (brittle situation). Maybe we need to retinker this class a bit to make it easier to test?
There was a problem hiding this comment.
Yeah, I felt a little dirty calling "private" directly, but less dirty than trying to test the big ole outer method. Not sure I see a clear path to retinker. I was just happy to manage to cover all the cases. ;)
| ); | ||
|
|
||
| expect(result).to.equal(false); | ||
| expect(written.indexOf(' [32mPath: supPath[39m\n')).to.not.eq(-1); |
There was a problem hiding this comment.
Holy guac, what is the [32m' stuff?
There was a problem hiding this comment.
I assume it is color stuff
| expect(result).to.equal(false); | ||
| }); | ||
|
|
||
| it('should include supplemental when empty', () => { |
There was a problem hiding this comment.
Both of these tests fail for me locally, currently.
There was a problem hiding this comment.
Could you share the failure info?
| expect(written.indexOf(' [32mRequired By: [39m\n')).to.not.eq(-1); | ||
| }); | ||
|
|
||
| it('should include supplemental', () => { |
There was a problem hiding this comment.
Both of these tests fail for me locally, currently.
This is what you get when a noob tries to add some unit tests.
If not too objectionable, these could be merged to the original PlayingWithRequiredBy branch.
Suggestions welcome.
cc @bhamail / @DarthHater / @allenhsieh / @ken-duck