Improve `pkg-tests.sh` to print the GAP output immediately, and not first into a log file

[fingolfin]

Right now, pkg-tests.sh runs make testpackages, which logs all output into files. We only print those files later. But that means that in the case of test runs which time out (e.g. due to an endless recursion/loop), we see nothing, just the information by Travis that there was a timeout. In particular, we don't see where it hangs (in which of the three different test suite runs, and in which file and line)

It would IMHO be much nicer if each test was run and immediately printed its output, while running. That would also mean that one gets output soon for the first of the three runs through the test suite; i.e., I could already see that the first of three test suite runs passed/failed, before the other two are done (granted, that's not something very important, it's just another bonus of such a change).

Technically, there are several ways to do this. We could modify testpackages, but my understanding is that @alex-konovalov uses this elsewhere.

The easiest would be IMHO to just not use testpackages anymore, and just run the tests directly, it's not hard after all. This is what the Makefile rule for it: (UPDATE: accidentally had quoted testpackages target here, now corrected)

testpackage: all
    $(MKDIR_P) dev/log
    ( echo 'SetAssertionLevel( 2 ); ReadGapRoot( "tst/testutil.g" ); \
            SaveWorkspace( "wsp.g" );' | $(TESTGAP) )
    ( echo 'CreatePackageTestsInput( "testpackage.in", \
            "dev/log/testpackage1", \
            "$(TESTGAP) -L wsp.g", "false", "$(PKGNAME)" );'\
            | $(TESTGAP) -L wsp.g > /dev/null )
    ( chmod 777 testpackage.in; ./testpackage.in; rm testpackage.in )
    ( rm wsp.g )
    ( echo 'SetAssertionLevel( 2 ); ReadGapRoot( "tst/testutil.g" ); \
            SaveWorkspace( "wsp.g" );' | $(TESTGAPauto) )
    ( echo 'CreatePackageTestsInput( "testpackage.in", \
            "dev/log/testpackageA", \
            "$(TESTGAPauto) -L wsp.g", "auto", "$(PKGNAME)" );'\
            | $(TESTGAPauto) -L wsp.g > /dev/null )
    ( chmod 777 testpackage.in; ./testpackage.in; rm testpackage.in )
    ( rm wsp.g )
    ( echo 'SetAssertionLevel( 2 ); ReadGapRoot( "tst/testutil.g" ); \
            SaveWorkspace( "wsp.g" );' | $(TESTGAP) )
    ( echo 'CreatePackageTestsInput( "testpackage.in", \
            "dev/log/testpackage2", \
            "$(TESTGAP) -L wsp.g", "true", "$(PKGNAME)" );'\
            | $(TESTGAP) -L wsp.g > /dev/null )
    ( chmod 777 testpackage.in; ./testpackage.in; rm testpackage.in )
    ( rm wsp.g )

Since we only test a single package each time, we don't need to bother with creating workspaces. I also see no benefit in using CreatePackageTestsInput (but I may be missing something?).

Instead, we could use TestOnePackage from https://github.com/gap-infra/gap-docker-pkg-tests-master-devel/blob/master/ci.g (and indeed, perhaps merge that function back into GAP's TestPackage). And then pipe a program like this into GAP:

result:=TestOnePackage("$PKG_NAME");
if result = true then FORCE_QUIT_GAP(0); fi;
FORCE_QUIT_GAP(1);

repeated three times with different args for GAP.

[olexandr-konovalov]

I am using testpackages, so I want a log as a file, but if one can use tee like in other targets, then I will be happy.

What you miss however is that there are two targets, testpackage and testpackages, and pkg-tests.sh uses

make testpackage PKGNAME=$PKG_NAME

The other target indeed needs to create workspace, because it is used again and again for each new package.

[fingolfin]

Yeah, I mixed up testpackages and testpackage (I've updated the description of this issue to correct that). But there is very little difference between them; in particular, testpackage also uses workspaces.

And as I wrote, I don't want to touch your "test" targets in Makefile.rules atall, I want to do it directly, which is IMHO much* simpler, and thus much easier to debug. I tried earlier to figure out what testpackage and testpackages do exactly but gave up after several levels of helper files being read, other files being generated, those being read again, more files being generated... :-)

[olexandr-konovalov]

testpackage was copypasted from testpackages, that's why. The latter uses helper files, the former could be certainly rewritten by @someone...

[fingolfin]

I could work on this (but not immediately, so if someone else is interested, by all means: go ahead!) But then it'd be nice to be able to test this without testing it with all packages. Like, I'd like to pick a few packages and then trigger the job with just that (could be as simple as: "just put the first 10 packages into the job matrix, then stop").

OK, typing this now, I realize that I can probably achieve this by editing the workflow file https://github.com/gap-infra/integration/blob/main/.github/workflows/pkg-tests.yml and changing the for PKG inls */PackageInfo.g ...` loop appropriately. OK, fine, that's enough for me then :-)

[wilfwilson]

Yeah, or just overwriting the matrix by hardcoding one after the for loop 🙂

I will look into allowing these workflows to be manually dispatched in a way that allows for only one package to be tested.

[wilfwilson]

@fingolfin Your can now run that Workflow file with just one package by manually dispatching it: https://github.com/gap-infra/integration/actions/workflows/pkg-tests.yml