Higher test coverage score needed to confidently accept pull requests
- Dominant language
- JavaScript
- Stars
- 9.3k
- Forks
- 1.4k
- Avg merge
- 6d 16h
- Merged PRs (30d)
- 1
Description
This issue has changed in focus — previously I thought there were a lot of unit tests that didn't actually test anything, but in reality they were setting up other specs. This has been documented as #2047 and is fixed with #2050.
Thus the focus of this issue now is to improve test coverage.
### If you want to help, please pick a file with low coverage [(latest list here)](https://codecov.io/gh/c3js/c3/tree/d5afd6677a2da7b99651773dbb096df50dfc095f/src) and file a PR improving it!
## The following is outdated and left merely for historical purposes
I think part of why we've had so many regressive bugs added is because we're not actually unit testing huge swaths of the codebase.
I ran into the following line while troubleshooting an issue for a user:
`expect(true).toBeTruthy()`
This assertion _will always pass_. And it's _littered_ throughout the codebase.
https://github.com/c3js/c3/search?utf8=%E2%9C%93&q=%22expect%28true%29.toBeTruthy%28%29%22&type=Code
If you run `grep "expect(true).toBeTruthy()" spec/* | wc -l`, you get `140`. That means of the ~400 assertions made across all unit tests, 140 _mean absolutely nothing_.
I repeat:
#140 of our assertions test _absolutely nothing_
This is just one invalid assertion. For all we know, even more unit tests don't actually test what they're intended to test but look like they do.
Part of this is how C3 was originally constructed — because of a lack of modularity and how interconnected each piece is (particularly due to a reliance on concatenation), it wasn't really that easy to import a particular part of the library and run unit tests against it — you had to deal with the entire processed library and any additional side effects caused by that.
I'm not sure what the best answer is here. My thought is to just bin any unit tests with `expect(true).toBeTruthy` in them so as to reduce the coverage score, convert everything to Mocha as per #1838, and then add the appropriate tests, gradually increasing test coverage.
Contributor guide
Assessment
This issue has not been assessed yet.