Defining and revising assertion levels

[hulpke]

This is prompted by @stevelinton 's remark in #562:

Since the standard tests now use assertions we should define what the different assertion levels achieve. As a start for discussion (copying from the suggestion for level 1):

Level 1: The tests should not cause more than 10% overhead in runtime and should not trigger calculations that might have side-effects, such as changing the state of the random number generator, storing attributes or setting properties. (This would imply that even an element test or a test for a group being solvable should be assertion level 2).

Level 2: Tests may cause side-effects but should not be expected to cause more than 50% overhead in runtime. It should be possible to run even longer-runtime tests with this assertion level being set.

Level 3: Tests that may increase runtime significantly, but will be safe to run (i.e. they cannot corrupt data structures or assume a set-up of data structures that is more specific than documented.

Level 5: Tests that may assume extra properties of the data structures or objects used which may not hold in general.

By this the homomorphism assertions should move to level 3 (and many other assertions to level 2)

[olexandr-konovalov]

@hulpke - thanks! My comments:

• Is there a reason why Level 4 is missing in the list?
• I think it may be hard to decide whether an assertion is of level 1 or of level 2 in advance, but at least that would give the rule: once we discover that the assertion have side-effects like that, we move it to level 2.
• Similarly, it's hard to measure the overhead in runtime in advance - in one example, it could be tiny, in another it may run out of time. So perhaps we should explain that we're speaking about the runtime of the (changeable) GAP test suite.
• We need to decide and recommend package authors at which assertion level their tests should run. At the moment, make testpackages uses Level 2, and START_TEST in packages test files enforces that. If START_TEST will not enforce Level 2 any more after Don't change assertion level in START_TEST and STOP_TEST. #549, many packages will provide tests prepared at level 0, and make testpackages may use Level 1 to run them. I think then this is a change for the next major release, as it may break package tests just by altering their output.
• If homomorphism assertions will be moved to level 3, this may resolve Performance regression in Alnuth #562 and permit us to proceed with the public release of GAP 4.8, continuing other tests at Level 2 as we do now, so I am in favour of such change.

[hulpke]

@alex-konovalov

• Level 4 was skipped in case we want to differentiate between two classes of hard tests that always work.
• I think the exclusion of side effects means that a level 1 assertion can only be for properties of basic objects -- particular values of integer/boolean variables, Rank of a matrix etc.
• Measuring the overhead exactly would be hard, but I think a routine's author would have an idea roughly how expensive a calculation is. If estimates are badly off we can always rank higher.

[stevelinton]

These are guidelines, not rigid rules, and it is very easy to change the level of an assertion in a fix if it is found to be too low (or too high).

Tests should pass at all assertion levels up to 4, although 4 might take a long time and package and library code should be tested at all these levels at some stage before a release.

Based on the criteria above, level 1 would be appropriate for the continuous integration testing run on every PR. Level 2 should be sensible for nightly testing and levels 3 and 4 are probably saved for release preparation.

Level 5 allows assertions that may only be appropriate in specific contexts, so is never used for automated testing.

[olexandr-konovalov]

I think this should be documented somewhere, and this will allow us to close the issue. Practical parts would be merging #549 in combination with setting assertion levels globally, perhaps via implementing #527.

[fingolfin]

However, I don't think this needs to be in 4.9 -- we want to get that out soon, and there are other more important things we should take care of first. So I propose to remove the milestone, or change it to 4.10.

[olexandr-konovalov]

@fingolfin agree - was going to suggest the same by same reasons. Will put 4.10.