pboettch / json-schema-validator

JSON schema validator for JSON for Modern C++
Other
512 stars 145 forks source link

Unit testing framework #266

Open LecrisUT opened 1 year ago

LecrisUT commented 1 year ago

How about using catch2 or any other unit testing frameworks. Nlohmann seems to be using doctest for that, probably because of the limitations it had when catch2 was header-only, but these days I don't think they are relevant anymore and catch2 has made quite nice cmake integrations for it.

pboettch commented 1 year ago

Instead of my custom bash-script plus ctest?

The problem is the json-schema-validator-test-suite. This is imported here and I really like the idea of reading from files, the schema and the instance.

LecrisUT commented 1 year ago

Hmm, so all unit testing is through test-suite? I was thinking of the individual folders over there. We could have both, where catch can handle anythings that is not covered by json-schema-validator-test-suite.

I think the test-suite should be converted to a submodule so it is easier to keep up to date. I can design something custom using either the proper cli interface or using catch2. The latter will be easier to create fine-grained control in the future e.g. experimental drafts.

pboettch commented 1 year ago

There is no unit-testing. Only functional testing. By using the test-suite and, in a similar manner (files) some test-cases which were mostly issues seen during the life of the project.

What is really missing is a coverage analysis, which points us to the weaknesses not covered by issues and functional tests.

pboettch commented 1 year ago

I strongly believe we can achieve 100% coverage by using the test-suite, issue-test and some corner case test written by hand. At this state of the library.

LecrisUT commented 1 year ago

Well, I've enabled the coverage, and indeed it was showing 90℅ coverage. Will have to wait for it to be merged in main for the full report though. After that let's double check if we need any additional testing framework or if we cover it all via test-suite.

But about test-suite, what about making it a submodule?

pboettch commented 1 year ago

Why not use fetchcontent to get the test-suite, if required (ie. enabled via cmake)?

LecrisUT commented 1 year ago

Oh, interesting point. I thought fetchcontent is only usable for cmake projects, but let me do a bit of researching and I'll get back to you on this.

LecrisUT commented 1 year ago

Thanks for the idea. FetchContent does indeed work for that. There is one design problem though, how to make this compatible with Fedora packaging. For that, it must be buildable without internet connection.

My thought is to still use git submodules and make a release GH action that includes those submodules. Then on the cmake side, I can make a simple check if there is internet connection and if not use the submodule location as a fallback. Or, keep the logic simple (always defaulting to downloaded FetchContent and fail if no internet connection) and keep it as an advanced configuration method (manually setting FETCHCONTENT_SOURCE_DIR_TESTSUITE).

I can make a simple script to add to pre-commit so that the submodule version and the FetchContent one are always in sync.

pboettch commented 1 year ago

Or we leave it as it is and just copy the test-suites (for the supported version) here from time to time ;-)

LecrisUT commented 1 year ago

True, still needs to be redesigned so that the contents are untouched and it is easier to update it. (would also make it possible to add the tests as experimental in the CI ;) )