The names of plist TNUMs have changed in GAP master, breaking our tests

[wilfwilson]

The names of plists in GAP master have been changed to become more user-friendly; see gap-system/gap#3394. I think this is a good thing. For instance, list (plain,empty) has become empty plain list, and many much more horrible ones have been improved too. This causes test failures such as the ones shown below, from a Travis job on my fork.

The problem is that we can't just change to using the new output in our test files, because that means our tests will no longer pass in older versions of GAP, such as GAP 4.9 and 4.10, and I don't think we're ready to give up full compatibility yet will all released versions of GAP.

This is unfortunate.

What should we do? I see two options:

• Remove the tests that cause the diffs. Solves the immediate problem cleanly, but reduces code coverage.
• Change the error message so that we no longer print the name of the unexpected object. This solves the problem but arguably makes the error message less useful.

What do you think @james-d-mitchell?

testing: /home/travis/gap/pkg/digraphs/tst/standard/grahom.tst
########> Diff in /home/travis/gap/pkg/digraphs/tst/standard/grahom.tst:51
# Input is:
HomomorphismDigraphsFinder(gr1, gr2, fail, [], 1, 1, 1, [1, []], 0, 0, 0);
# Expected output:
Error, the 8th argument <image> must only contain positive integers, but found\
 list (plain,empty) in position 2,
# But found:
Error, the 8th argument <image> must only contain positive integers, but found\
 empty plain list in position 2,
########
########> Diff in /home/travis/gap/pkg/digraphs/tst/standard/grahom.tst:55
# Input is:
HomomorphismDigraphsFinder(gr1, gr2, fail, [], 1, 1, 1, [[], []], 0, 0, 0);
# Expected output:
Error, the 8th argument <image> must only contain positive integers, but found\
 list (plain,empty) in position 1,
# But found:
Error, the 8th argument <image> must only contain positive integers, but found\
 empty plain list in position 1,
########
Abort: DigraphsTestStandard failed . . . 

[ChrisJefferson]

I might have a plan, we could update GAP's kernel version, then you could #@if the test based on that.

[wilfwilson]

Is the #@if functionality backwards compatible with GAP 4.9 and GAP 4.10?

[wilfwilson]

Oh, are you talking about a C-level check rather than your cool GAP test file-related checks?

[ChrisJefferson]

Ah yes, probably not backwards compatable enough.. damn. Could use that in future.

[james-d-mitchell]

How many tests are we talking about? If it's not that many, then we could just comment them out. The actual text of the error messages is not really very important, only that an error is given with a particular input. Is it possible to check in GAP whether or not a given command errors? I'm thinking of something like Errors("a" := "b"); or something similar. The best fix would be to make the test independent of the actual error messages, and just to check that an error is given instead.

[james-d-mitchell]

Doesn't CALL_WITH_CATCH do something like this?

[wilfwilson]

It's just two tests, at least I think so (the ones I pasted into the description of the issue). So for now we should maybe just comment them out.

I think that the main purpose of testing errors is making sure that an error is given. But I also I think that it's least somewhat important to test that the error message is actually meaningful and appropriate. So I'm not sure what the best solution would be, overall.

[wilfwilson]

This was closed automatically by the PR. But I'm aiming to make a release imminently.