-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposals for the test system #39
Comments
I more than happy to looking at remodeling the tests. I agree there is lots of stuff not quite right. Some points
|
I'd also remove dependencies to libdivsufsort. We're currently adopting the code of libdivsufsort to a header-only-friendly format so that the code can be part of the sdsl without any pre-compiling, linking, etc. (which should be fine since it is published under the MIT license). Then we also wouldn't need any install scripts. Installing the header files could be done by cmake itself. |
|
I would propose a few changes to the test system:
Building tests creates new files in the repository.
Since building a
*.cpp.cmake
file results in copying the file to*.cpp
and inserting the types, it adds these files to the repo which doesn't seem to be a good style. Also changes to bits.hpp, uint128.hpp and structure_tree.hpp whenever including the library, make it confusing when working on library and committing code (but this is a separate issue). This could be fixed easily by only having one test suite (see my next point) and moving the types from*.typedef
into*.cpp
which would make*.cpp.cmake
files obsolete.Here is a short article on separating build directories from the source folder.
There are currently two test suites (test-sdsl and test-full-sdsl)
I don't see a good reason for having a sparse test set. Changes in the library will in most cases be covered by a single typed test case (for debugging and local testing) and when a pull request is opened, the full test set should be run by CI anyway. I would propose to merge all types (and input files) into one test suite, include ccache for caching builds on TravisCI (which would reduce the build time further) and if necessary, ask TravisCI for an extension of the max. build time (last time we checked they did it for free for open source projects). We would then also include the SDSL in our Jenkins CI (for nightly builds and pull requests) which does not have a time limit.
Building and execution of tests is currently done in an interleaved fashion
When changes in the code somewhere results in a compiler error, it probably should be fixed first before one might start debugging runtime errors elsewhere.
One test execution failing prevents building (and executing the rest of the test suite)
Even if the tests wouldn't be interleaved anymore, it would be great if all tests were built and executed (even if some of them failed) to have a summary in the end which test cases are broken (in terms of compilation and runtime erorrs).
What are your views on this? I can prepare a pull request for this if you're interested/agree with these points. :-)
The text was updated successfully, but these errors were encountered: