Increase code quality by increasing coverage: lib/report.js

[mcknasty]

Pull Request 452

This pull request has two commits.

Increase code quality by increasing coverage: lib/report.js

Expose the Report Class for future extension

Refactoring the report module export was necessary for testing

The purpose of this pull request is to increase code coverage for the file lib/report.js. This file was somewhat challenging, as certain line numbers were not covered only on certain versions of node.js. Furthermore, there were line numbers that were covered in other code coverage patches.

On Node.js versions 10.24.1 or 12.22.12 I was not able to get 100% coverage. On version 12.22.12 I was having some challenges getting this particular line covered by c8. It appears to be only a }, and the error is in branching. Version 10.24.1 seems to follow a different coverage model where reports vary greatly from other versions.

lib/report.js Code Coverage Matrix Report

Test Type	File	Statement	Branch	Function	Lines	PASS
Final Test	lib/report.js	100%	100%	100%	100%	✔️
Node 10.24.1	lib/report.js	90.58%	80.39%	86.66%	90.58%	❌
Node 12.22.12	lib/report.js	100%	98.48%	100%	100%	❌
Node 14.21.2	lib/report.js	100%	100%	100%	100%	✔️
Node 16.19.0	lib/report.js	100%	100%	100%	100%	✔️
Node 18.14.0	lib/report.js	100%	100%	100%	100%	✔️

Following the Contributions Recommendations here.

1. Make sure you have installed the latest version of Node.js
2. Fork this project on GitHub and clone your fork locally
3. Create local branches to work within. These should also be created directly from the main branch. Local Fork here.
4. Make your changes.
5. Run tests to make sure all is okay (everything should pass except the snapshot).
   1. A complete log of initial test results.
   2. As instructed, ignore snapshot failures. 1 failing
   3. 54 passing in 28 seconds
6. Now update the snapshot.
   1. A complete log of snapshot test results.
   2. 55 passing in 28 seconds
7. If all is passing, commit your changes. Log of commit can be found here.
8. As a best practice, once you have committed your changes, it is a good idea to use git rebase (not git merge) to synchronize your work with the main repository.
9. Run tests again to make sure all is okay.
   1. A complete log of the final test results.
   2. 55 passing in 30 seconds
10. Push
11. Open the pull request, see details in the template.
12. Make any necessary changes after review.

Unit Tests Results

Test Type	PASS	Tests Passed	Tests Failed	Time
Initial Test	❌	54 passing	1 failing	28 seconds
Snapshot Test	✔️	55 passing	0 failures	28 seconds
Final Test	✔️	55 passing	0 failures	30 seconds

Node Version Testing Matrix

Node Version	PASS	Tests Passed	Tests Failed	Time
10.24.1	✔️	54 passing	0 failures	50 seconds
12.22.12	✔️	55 passing	0 failures	50 seconds
14.21.2	✔️	55 passing	0 failures	40 seconds
16.19.0	✔️	55 passing	0 failures	41 seconds
18.14.0	✔️	55 passing	0 failures	38 seconds

Referencing Issue #448

[bcoe]

Left a note about testing "private" methods.

[bcoe]

These are private methods, rather than forcing them to throw, it would be better if we could figure out a condition that caused loadReports to throw an exception, when invoked through the public interface.

tldr; it's better if the tests describe an understandable beahviour, e.g., "when reports are in this format, an exception is thrown".

[mcknasty]

These are private methods, rather than forcing them to throw, it would be better if we could figure out a condition that caused loadReports to throw an exception, when invoked through the public interface.

Conditions that would cause the exception to be thrown will be discovered over time through bug reports.

From my perspective, it can be difficult to tell what application state can cause bugs. That is why they are bugs. It's an application state that you didn't anticipate, and there is no logic written for that application state. In certain cases, it may be an invalid application state or at other times it could be a feature not working as intended. It's important to test that exceptions are thrown and working as expected, then as bugs arise, hopefully, it's covered by an exception.

[mcknasty]

I am working on some additional enhancements. I am not excited about how cumbersome chai-spies are, so I am researching other test investigation modules. Currently, there is only one line that is not getting covered in this current patch.