-
Notifications
You must be signed in to change notification settings - Fork 162
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
Improve coverage of the test suite #314
Comments
@ChrisJefferson I think it's now a hot time to put your profiling package on deposit - it would be useful by many reasons to distribute it with GAP 4.8 beta, not waiting until the first public GAP 4.8. By the way, do we have a way to consolidate test coverage results from several test sessions? For example, the union of |
@ChrisJefferson furthermore, @fingolfin certainly we could set up Jenkins job to run the coverage test e.g. weekly and then publish them somewhere. I suggest that the first step towards this should be actual depositing of @ChrisJefferson's package. |
Of course, I fully agree that the testing setup and coverage needs to be improved. Important aspects for me would be:
|
Can't agree more with the four items below. Tried to collect some current and desirable things:
This was a strong design guideline followed all the time since making GAP 4.5. Many steps in the testing and releasing workflow are VCS and CI agnostic, in particular:
|
There are a lot more things it would be good to be able to do at GAP level than It would also be nice to have a finer structure than a whole test file, so that a file could contain many tests which could pass and fail independently (and perhaps also have independent timings etc.). Ideally you want to see something like "97 out of 103 tests in this file passed" and then drill down to see (personally or from a programme) the details of the failures -- "sum of mutable vectors was not mutable", "break loop executing such and such a command", "no break loop executing some other command", "no result returned when one was expected", etc. |
This kind of thing shouldn't be too hard to add, it's mainly a matter of file format! There are in my mind two broad options:
Seeing as the existing testing format allows comments in files, it wouldn't be hard to have some simple special formatted lines to split files up: Perhaps something simple like "Start a line with ## to denote the start of a new block of tests", so you would write something like:
|
@ChrisJefferson: My approach would be 2) (Extend the existing testing format). I would write utilities in GAP on top of One idea: Using the utilities for code coverage one could determine (and cache) for each test file the list of code files containing code that is used during these tests. After editing some files one could then run exactly those tests which are relevant for the changed files. |
My approach actually would be both 1) and 2). There are some tests for which the existing test format are nice, and improving it with annotations would be helpful. But there are also some kinds of test for which I find it very awkward and cumbersome to use the existing test format; e.g. for the tests in the recog package, rewriting them in the existing test format would IMHO quite annoying. This is the main reason why I wrote the On another note, I think the Homalg people have yet another system, where they put tests into comments in their source files, and then extract them from there to be run?!? or so?!? perhaps @sebasguts could elaborate? |
I do not think our approach is new here. We test manual examples. Sometimes we put them in the source code, see for example We also have a package ExamplesForHomalg, which we use for further testing. But those tests are far away from the source code and almost certainly not what you have in mind here, right? |
For the record, so do I in my packages - see e.g. wedderga/makedocrel.g. Whenever I build the manuals with this script, it also extracts all examples and writes them into test files (one per chapter) in the |
Testing examples in the manual is certainly one sensible test. But manual examples are usually not extensive tests for the code in question, and vice versa an extensive test suite for some code should not be put in manual examples. If people want test code close to the implementation one could generate test files in the same way as the manual is collected from source (and other) files. E.g., using |
@frankluebeck of course that's true - it's a way to easily generate test files for the package and to ensure that examples within a chapter run 100% if they are in the same order, but not much more. Using |
You can see the current coverage of both the C and GAP code at: https://caj.host.cs.st-andrews.ac.uk/gap-cover/ This output is generated using the scripts (and vagrant machine) at: https://github.com/ChrisJefferson/GAPCoverage These scripts build a custom version of GAP. This is needed for the coverage of the C code. |
@ChrisJefferson Thanks! Is is documented somewhere how to read this, e.g. what different colours in the GAP code coverage report mean? |
Some documentation would be good, more is coming :) Green (Blue in the C code ) = line read and executed In short -- we want no red on either profile. |
We need to run this on a regular basis and put the results on a webpage though, or are you doing that already? |
I could set up a Jenkins build, but hardly earlier than November 24th. |
We've done most of this -- although it might still be nice to have a new test format. |
I think one of the major problems in GAP development is that we lack a comprehensive test suite. Hence, any given change, no matter how innocent it may appear, can break functionality in GAP, and it may take us months until we notice it, say after a user reports an issue.
Let's try to fix that. We discussed this often enough, so, I'll try to summarize what I think the conclusion of those discussions is. This issue here then could be considered a "meta-issue", tracking the overal progress, with the actual work done elsewhere.
UnitTests
package was an experiment in that direction, and I feel there are some other ideas we could explore in this regard. But that should not be discussed here, but rather on a separate issue and/or the mailing lists, and/or in live meetings.Perhaps some people can also get fund for this, and hire some students to help out with this. Let's be creative!
The text was updated successfully, but these errors were encountered: