Start building unit tests and improve code coverage. about schemaspy HOT 10 OPEN

I'll help with this. It might get ugly but having tests before refactoring is required.

from schemaspy.

@rafalkasa how do you want to handle test failures?

Example utils/Caseinsensativemap will fail it's purpose if entries are added with methods that are not overriden example putIfAbsent.

In this case the full Map interface is exposed. PR with failing tests or issue to discuss solutions.

From a verify functionality I would probably create a facade interface to avoid implementation of Map interface. Since Map interface isn't owned by schemaspy it needs to evolve with the jdk which might be a coupling one wouldn't want.

from schemaspy.

@npetzall you need to know that a lot of code is more than ten years old in this project.
Maybe instead of thinking how to test utils\CaseInsensitiveMap class as it is we should start thinking about refactor and find out better solution to doing the same.

The same is with other part of the project first we need to think how to make this code testable and after try to write the unit test to not avoid lost some functionality.

From other point of view we should concentrate to test this part of the project what is the core one and where the most complex application logic exist.

from schemaspy.

@npetzall if you want to improve project unit test coverage please propose something and during the review your PR we can discuss about the details if your solution and test will be ok or not 😄

from schemaspy.

@rafalkasa testing legacy code is actually something challenging and for me a bit fun. But in a refactoring sense it should be done first, verify and inspect behaviour so that you can verify said behaviour after refactoring.

The case with Caseinsensativemap is that I've found holes in the implementation, not that I have problems with testing.

Same goes with mocking, don't want to do to much of it in the beginning since it would decrease test coverage. So first build coverage then refactoring and mocking.

I've only looked at the project these last couple of days. So if you provide a "hit" list I'll attack said list with test.

from schemaspy.

@npetzall the level of code coverage looks impressive mostly because of you.
So we can close the issue, and maybe set Travis to not pass the PR build of code coverage is less than 80% or more

from schemaspy.

I don't think that Sonarqube is run for PR unless the github apps will allow it.

from schemaspy.

Well, for local, but not for PR coming from a fork.

from schemaspy.

Maybe we can use other coverage solution for this verification which is working with fork PRs

from schemaspy.

This issue with Sonarqube and GitHub forks PR is very old topic as I see nothing change..

from schemaspy.