zfp icon indicating copy to clipboard operation
zfp copied to clipboard

Full test suite not in release

Open tpwrules opened this issue 3 years ago • 2 comments

For some reason, the full test suite is not part of the release tag or branch. This makes it difficult to test that a distributed copy works correctly. I can't figure out why this is, but I would appreciate it being corrected.

tpwrules avatar Aug 27 '22 18:08 tpwrules

The full test suite appears on the release1.0.0 branch, which is pruned for master and all tagged releases.

There are some legacy reasons for excluding the majority of tests with releases. Some of our software development practices have been shaped by the desire to minimize external dependencies, to make zfp as easy to build as possible, and to simplify including zfp in other projects. Toward this end, we have made a number of decisions/compromises:

  1. Back when zfp was distributed mostly as tar balls on institutional web servers, we decided to exclude most unit tests to reduce repo size and build/test time and to make including zfp source directly in other projects more attractive. These tests, which are primarily of interest to zfp developers, make up more than half of the repo today but are not needed by most users.
  2. Unit tests add dependencies, such as GoogleTest and cmocka. The purpose of these tests is primarily to catch bugs during development and less to ensure correctness when building zfp in a particular environment. To be sure, both are important, but the vast majority of issues are caught during development and extensive CI testing. When necessary, it's always possible to perform full testing using the develop and release branches.
  3. zfp has deliberately been written for C89 and C++98 for maximum portability and ease of adoption. The tests currently require C99 (because we took shortcuts when writing them), C++11 (because GoogleTest does), and CMake, which add an extra level of dependency not required by the main library.

This all being said, zfp has evolved a lot over the past few years and now includes multiple backends (OpenMP, CUDA, HIP) and language bindings (C, C++, Fortran, Python), some of which add their own dependencies when enabled. Furthermore, zfp is now more dependent on GitHub and various CI services, and with new code complexity comes more advanced testing requirements. So, there's a good case to be made for including the full test suite with future releases to ensure adequate code coverage.

We will re-evaluate how to best manage testing in future releases. For now, any previous release can be tested using the corresponding release branch.

lindstro avatar Aug 28 '22 05:08 lindstro

Thank you for the information on finding the corresponding tests and background on why these decisions were made. I did observe that "more than half" of the repo is indeed tests (and docs), but IMHO an increase of the distribution from 200 kilobytes to 500 kilobytes is not a significant burden in today's world :)

I think it would be better if these development tests were hidden behind a CMake flag rather than made available by using a different branch. Having the release commit be directly connected to the development branch instead of the subject of some undocumented pruning is also good for clarity.

tpwrules avatar Aug 28 '22 15:08 tpwrules

@tpwrules commit fc96c91 merges in changes that should address this. The BUILD_TESTING option still exists and when enabled will build a subset of the tests. These tests should be sufficient for ensuring correctness. However, we will no longer prune the full test suite from releases and now provide a BUILD_TESTING_FULL option for running it if desired. Hopefully this should provide you with the desired functionality.

GarrettDMorrison avatar Oct 13 '22 21:10 GarrettDMorrison