Diffs in AtlasRep standard tests

[olexandr-konovalov]

The log below comes from Jenkins CI. It happens that AtlasRep's tst/testall.g file contains some tests that does not work when TERM = "dumb", the setting used in Jenkins CI.

Those tests pass fine when tst/testall.g is read into a usual GAP session, and do not take much time. They automatically open and close some Browse package windows, and I understand that the developers of AtlasRep would like to keep them in tst/testall.g to run regularly. Therefore, it seems for me that the best solution would be to omit them if TERM = "dumb" and run otherwise.

Additionally, tst/testall.g calls Test, but does not analyse its returned value. This does not permit automated detection of test failures. As said in https://github.com/gap-system/gap/wiki/Status-of-standard-tests-in-GAP-packages, the automated detection of the result of the test is relying on lines containing the substrings

#I  Errors detected while testing

or

#I  No errors detected while testing

(with exactly two spaces after #I), dependently on the test result. All that is needs is to calculate testresult and add the following lines

if testresult then
  Print("#I  No errors detected while testing\n");
else
  Print("#I  Errors detected while testing\n");
fi;

to the end of the test.

The mentioned log file is below.

============================OUTPUT START==============================

#I  RunPackageTests("atlasrep", "1.5.1", "tst/testall.g", auto);
 ���������������������������   GAP 4.8.8, 20-Aug-2017, build of 2017-08-20 19:40:04 (BST)
 ���  GAP  ���   https://www.gap-system.org
 ���������������������������   Architecture: x86_64-pc-linux-gnu-gcc-default64
 Libs used:  gmp, readline
 Loaded workspace: wsp.g
 Components: trans 1.0, prim 2.1, small* 1.0, id* 1.0
 Packages:   AClib 1.2, Alnuth 3.0.0, AtlasRep 1.5.1, AutPGrp 1.8, 
             Browse 1.8.7, CRISP 1.4.4, Cryst 4.1.12, CrystCat 1.1.6, 
             CTblLib 1.2.2, FactInt 1.5.4, FGA 1.3.1, GAPDoc 1.6, IO 4.4.6, 
             IRREDSOL 1.4, LAGUNA 3.7.0, Polenta 1.3.7, Polycyclic 2.11, 
             RadiRoot 2.7, ResClasses 4.6.0, Sophus 1.23, SpinSym 1.5, 
             TomLib 1.2.6, Utils 0.46
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'
########> Diff in /data/gap-jenkins/workspace/GAP-minor-release-test/GAPCOPTS/\
64build/GAPGMP/gmp/GAPTARGET/packages/label/kovacs/gap4r8/pkg/atlasrep/tst/doc\
xpl.tst, line 871:
# Input is:
if IsBound( BrowseMinimalDegrees ) then
  down:= NCurses.keys.DOWN;;  DOWN:= NCurses.keys.NPAGE;;
  right:= NCurses.keys.RIGHT;;  END:= NCurses.keys.END;;
  enter:= NCurses.keys.ENTER;;  nop:= [ 14, 14, 14 ];;
  # just scroll in the table
  BrowseData.SetReplay( Concatenation( [ DOWN, DOWN, DOWN,
         right, right, right ], "sedddrrrddd", nop, nop, "Q" ) );
  BrowseMinimalDegrees();;
  # restrict the table to the groups with minimal ordinary degree 6
  BrowseData.SetReplay( Concatenation( "scf6",
       [ down, down, right, enter, enter ] , nop, nop, "Q" ) );
  BrowseMinimalDegrees();;
  BrowseData.SetReplay( false );
fi;
# Expected output:
# But found:
#I  cannot switch to visual mode because of TERM = "dumb"
#I  cannot switch to visual mode because of TERM = "dumb"
########
########> Diff in /data/gap-jenkins/workspace/GAP-minor-release-test/GAPCOPTS/\
64build/GAPGMP/gmp/GAPTARGET/packages/label/kovacs/gap4r8/pkg/atlasrep/tst/doc\
xpl.tst, line 887:
# Input is:
if IsBound( BrowseMinimalDegrees ) then
  # just scroll in the table
  BrowseData.SetReplay( Concatenation( [ DOWN, DOWN, DOWN, END ],
         "rrrrrrrrrrrrrr", nop, nop, "Q" ) );
  BrowseMinimalDegrees( BibliographySporadicSimple.groupNamesJan05 );;
fi;
# Expected output:
# But found:
#I  cannot switch to visual mode because of TERM = "dumb"
########
########> Diff in /data/gap-jenkins/workspace/GAP-minor-release-test/GAPCOPTS/\
64build/GAPGMP/gmp/GAPTARGET/packages/label/kovacs/gap4r8/pkg/atlasrep/tst/doc\
xpl.tst, line 895:
# Input is:
if IsBound( BrowseBibliographySporadicSimple ) then
  enter:= NCurses.keys.ENTER;;  nop:= [ 14, 14, 14 ];;
  BrowseData.SetReplay( Concatenation(
    # choose the application
    "/Bibliography of Sporadic Simple Groups", [ enter, enter ],
    # search in the title column for the Atlas of Finite Groups
    "scr/Atlas of finite groups", [ enter,
    # and quit
    nop, nop, nop, nop ], "Q" ) );
  BrowseGapData();;
  BrowseData.SetReplay( false );
fi;
# Expected output:
# But found:
#I  cannot switch to visual mode because of TERM = "dumb"
########
Input file: docxpl.tst
GAP4stones: 316
Input file: atlasrep.tst
GAP4stones: 1005
########> Diff in /data/gap-jenkins/workspace/GAP-minor-release-test/GAPCOPTS/\
64build/GAPGMP/gmp/GAPTARGET/packages/label/kovacs/gap4r8/pkg/atlasrep/tst/doc\
xpl.tst, line 871:
# Input is:
if IsBound( BrowseMinimalDegrees ) then
  down:= NCurses.keys.DOWN;;  DOWN:= NCurses.keys.NPAGE;;
  right:= NCurses.keys.RIGHT;;  END:= NCurses.keys.END;;
  enter:= NCurses.keys.ENTER;;  nop:= [ 14, 14, 14 ];;
  # just scroll in the table
  BrowseData.SetReplay( Concatenation( [ DOWN, DOWN, DOWN,
         right, right, right ], "sedddrrrddd", nop, nop, "Q" ) );
  BrowseMinimalDegrees();;
  # restrict the table to the groups with minimal ordinary degree 6
  BrowseData.SetReplay( Concatenation( "scf6",
       [ down, down, right, enter, enter ] , nop, nop, "Q" ) );
  BrowseMinimalDegrees();;
  BrowseData.SetReplay( false );
fi;
# Expected output:
# But found:
#I  cannot switch to visual mode because of TERM = "dumb"
#I  cannot switch to visual mode because of TERM = "dumb"
########
########> Diff in /data/gap-jenkins/workspace/GAP-minor-release-test/GAPCOPTS/\
64build/GAPGMP/gmp/GAPTARGET/packages/label/kovacs/gap4r8/pkg/atlasrep/tst/doc\
xpl.tst, line 887:
# Input is:
if IsBound( BrowseMinimalDegrees ) then
  # just scroll in the table
  BrowseData.SetReplay( Concatenation( [ DOWN, DOWN, DOWN, END ],
         "rrrrrrrrrrrrrr", nop, nop, "Q" ) );
  BrowseMinimalDegrees( BibliographySporadicSimple.groupNamesJan05 );;
fi;
# Expected output:
# But found:
#I  cannot switch to visual mode because of TERM = "dumb"
########
########> Diff in /data/gap-jenkins/workspace/GAP-minor-release-test/GAPCOPTS/\
64build/GAPGMP/gmp/GAPTARGET/packages/label/kovacs/gap4r8/pkg/atlasrep/tst/doc\
xpl.tst, line 895:
# Input is:
if IsBound( BrowseBibliographySporadicSimple ) then
  enter:= NCurses.keys.ENTER;;  nop:= [ 14, 14, 14 ];;
  BrowseData.SetReplay( Concatenation(
    # choose the application
    "/Bibliography of Sporadic Simple Groups", [ enter, enter ],
    # search in the title column for the Atlas of Finite Groups
    "scr/Atlas of finite groups", [ enter,
    # and quit
    nop, nop, nop, nop ], "Q" ) );
  BrowseGapData();;
  BrowseData.SetReplay( false );
fi;
# Expected output:
# But found:
#I  cannot switch to visual mode because of TERM = "dumb"
########
Input file: docxpl.tst
GAP4stones: 322
Input file: atlasrep.tst
GAP4stones: 1107
#I  RunPackageTests("atlasrep", "1.5.1", "tst/testall.g", auto): runtime 81958
============================OUTPUT END================================

[olexandr-konovalov]

Also notified @ThomasBreuer by email.

[olexandr-konovalov]

I have found a setting (calling the test with TERM=xterm) which removes all #I cannot switch to visual mode because of TERM = "dumb" messages.

Now in the stable-4.8 branch AtlasRep tests pass (https://travis-ci.org/gap-system/gap-docker-pkg-tests-stable-staging/jobs/331189400) but that can not be detected automatically because of the reasons explained above.

In the stable-4.9 branch tests status is as follows:

With no packages loaded (GAP started with -r option) :  4 DIFFS
With all packages loaded with LoadAllPackages()      :  2 DIFFS
With default packages loaded at GAP startup          :  2 DIFFS

with diffs being

########> Diff in /home/gap/inst/gap-stable-4.9/pkg/atlasrep/tst/docxpl.tst:61
# Input is:
NrMovedPoints( Image( hom ) );
# Expected output:
1540
# But found:
266
########
########> Diff in /home/gap/inst/gap-stable-4.9/pkg/atlasrep/tst/docxpl.tst:
413
# Input is:
StructureDescription( stab );
# Expected output:
"(A4 x A4) : C4"
# But found:
"(C2 x C2 x C2 x C2) : ((C3 x C3) : C4)"
########

Full logs can be viewed at https://travis-ci.org/gap-system/gap-docker-pkg-tests-stable-4.9-staging/jobs/327912054

P.S. @ThomasBreuer reported the regression with StructureDescription in #2109.

[olexandr-konovalov]

The StructureDescription diff is triggered by not loading Crisp package (see #2109). In this case, if the point of the example is to check that stab has correct isomorphism type, one could replace it by

gap> IdGroup( stab );
[ 576, 8652 ] 

which should not depend on loaded packages.

[ThomasBreuer]

Of course not, we all should know that StructureDescription cannot be used to check isomorphism types.

The example is part of the tutorial, and the point is to express that the group has the claimed structure.
In this situation, a group ID is as useless as the new default StructureDescription output.

[olexandr-konovalov]

I see... Then perhaps the nice option, as @fingolfin suggested elsewhere, is suitable? The other diff in the example about NrMovedPoints seems to be a smaller degree permutation representation that GAP 4.9 will produce, so it just needs to be updated to the new output when you will be making a new release.

[olexandr-konovalov]

@ThomasBreuer AtlasReps tests still do not pass in the stable-4.10 branch, see for example the test log at https://travis-ci.org/gap-system/gap-docker-pkg-tests-stable-4.10-staging/jobs/438054207. For packages whose tests do not pass cleanly it's very easy to overlook when they become broken because of changes in GAP and/or other packages. Therefore, it will be very useful if AtlasRep tests can be fixed.

First, even the following small changes in testall.g will allow automated detection of test failures in CI scripts:

LoadPackage( "atlasrep" );

dirs:= DirectoriesPackageLibrary( "atlasrep", "tst" );

# Make sure that the component is bound to either `true' or `false'.
if not IsBound( CMeatAxe.FastRead ) or CMeatAxe.FastRead <> true then
  CMeatAxe.FastRead:= false;
fi;

testresult:=true;

# Run the standard tests with this value.
testresult := testresult and Test( Filename( dirs, "docxpl.tst" ) );
testresult := testresult and Test( Filename( dirs, "atlasrep.tst" ) );

# Now run the tests with the other value.
CMeatAxe.FastRead:= not CMeatAxe.FastRead;
testresult := testresult and Test( Filename( dirs, "docxpl.tst" ) );
testresult := testresult and Test( Filename( dirs, "atlasrep.tst" ) );

# Reset the value.
CMeatAxe.FastRead:= not CMeatAxe.FastRead;

if testresult then
  Print("#I  No errors detected while testing\n");
  QUIT_GAP(0);
else
  Print("#I  Errors detected while testing\n");
  QUIT_GAP(1);
fi;

FORCE_QUIT_GAP(1); # if we ever get here, there was an error

Second, there is currently a diff when GAP is started with -A option:

########> Diff in /home/gap/inst/gap-stable-4.10/pkg/atlasrep/tst/docxpl.tst:
413
# Input is:
StructureDescription( stab );
# Expected output:
"(A4 x A4) : C4"
# But found:
"(C2 x C2 x C2 x C2) : ((C3 x C3) : C4)"
########

It was discussed in #2109, and a "nice" option has been suggested there - is it suitable?

Note that now they are supposed to pass for 87 packages at https://travis-ci.org/gap-system/gap-docker-pkg-tests-stable-4.10 and fail for 12 at https://travis-ci.org/gap-system/gap-docker-pkg-tests-stable-4.10-staging, so the coming GAP 4.10 will have package tests in the state like never before. But there is still a space to reduce the latter number even further down to 10 or less in GAP 4.10.

[olexandr-konovalov]

@ThomasBreuer we have published GAP 4.10.0.

Unfortunately, no new AtlasRep release appeared, so as you can see at https://travis-ci.org/gap-system/gap-docker-pkg-tests, it is one of the 6 packages (out of 103) for which we can not report that their tests pass.

Furthermore, among those 6 packages there are two which are loaded by default (AtlasRep and CTbliLib) and it's quite unlucky that we do not check that they work as expected as thoroughly as for many other packages.

Therefore, are you happy with the testall.g file proposed above? Will you be able to publish a new release, say, this year, please? It could also address suggestions from #2237 and #2791. Do you have a repository for AtlasRep already? We will be happy to create one for you, and populate it with past releases (with one-to-one correspondence between releases and commits) - please let us know.