Skip to content
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

Replace custom test setup with TestDirectory #8

Closed
wants to merge 1 commit into from
Closed

Conversation

fingolfin
Copy link
Member

This PR removes the code in tst which generates test files from templates, and replaces the test runner by a "standard" one which uses TestDirectory. One advantage of this is that adding new tst files is just a matter of dropping them into the tst directory -- this way e.g. the tests added in PR #4 would be run automatically.

Another advantage is that this way we get better integration into the CI setup: TestDirectory reports success/failures via exit codes, which the test runner script here doesn't -- it only prints the messages indicating whether the test passed/failed, and we moved away from scanning the output log for those (it is too brittle).

I am not sure why the tests where setup the way they are right now. One guess is that this makes it easier to redo the tests if the output format changes. But that can also be achieved with the rewriteToFiles option to TestDirectory.

Another reason I've heard in the past for similar setups is that people want to use the "source" files tst/gap/test_forms*.g as standalone input, or copy&paster parts of it. Regardless, if you think there are reasons to keep them, this could also be done -- I could then amend this PR to leave this code in, and just replace tst/testall.g by my "standard" one.

I do not expect this PR to be simply merged, probably @jdebeule will at least have some questions or outright reject it.

But I thought I should at least demonstrate it as a proof of concept and explain why I think it's useful. I'd not be surprised if I miss part of the big picture, though, perhaps @jdebeule has some good reasons for the existing test setup that I am missing -- if so, it'd be good to learn them, then perhaps I can improve this PR to satisfy both of us? We'll see.

@jdebeule
Copy link
Collaborator

This looks fine, expanding the test files by just dropping new files into the correct directory is much more convenient than the way it is setup now. I just would like to add some information on the original setup, which was copied from another package (I forgot which one).

A "test file" consists of some GAP commands, stored in a .g file. To convert such a .g file into a .tst file (which obviously must contain GAP output as well), we had created "generate_tst_files.g" file. Starting from a list of .g files to be converted, this GAP script starts up a GAP session, reads a .g file, and makes sure the output is written in a correct way into a .tst file. No manual copy/paste needed to create a .tst file from a .g file. We use the same principle to generate example files for the documentation. Note that we have been struggling a bit with this: the generate_tst_files.g is a bit messy (to say the least), and I noticed that this script often malfunctions after an update of GAP. So a general question arises here: is there an easy way to convert .g files (for example those in ./tst/gap into .tst files to be used in the tst directory?

@ThomasBreuer
Copy link
Contributor

I would recommend to maintain only the tst/*.tst files, and to store the manual examples directly in the doc/*.xml files (or in the code files, if the rest of the documentation is stored there).
With the command AutoDoc("forms", rec(extract_examples:= true)); in makedoc.g, one can then generate test files from the manual examples, which will be processed automatically by TestDirectory.
The generated files need not be under version control.

If the idea was to keep the GAP input in separate *.g files because the output may be different in different GAP versions then one remedy would be to use the changeSources option of RunExamples to rewrite the source files such that the shown GAP output fits to what the used version of GAP shows.
Analogously, Test has a rewriteToFile option which can be used to adjust a test file to the version of GAP that is used for running the tests.

@jdebeule
Copy link
Collaborator

I see, but, after digging a bit in the documentation of GAPDoc and AutoDoc, I seem to conclude that all the functionality is meant to extract tst files from existing documentation. Therefore, I still have the following question. Assume that a file only containing gap commands is given, e.g. the file looks like

#test_forms1.g
f := GF(3);
gram := [[0,0,0,0,0,2],[0,0,0,0,2,0],[0,0,0,1,0,0],[0,0,1,0,0,0],[0,2,0,0,0,0],[2,0,0,0,0,0]]*Z(3)^0;
form := BilinearFormByMatrix(gram,f);
BaseChangeToCanonical(form);
Display(form);
TypeOfForm(form);
quit;

Then the example in the xml file will look like:

gap> f := GF(3);
GF(3)
gap> gram := [[0,0,0,0,0,2],[0,0,0,0,2,0],[0,0,0,1,0,0],[0,0,1,0,0,0],[0,2,0,0,0,0],[2,0,0,0,0,0]]Z(3)^0;
[ [ 0
Z(3), 0Z(3), 0Z(3), 0Z(3), 0Z(3), Z(3) ],
[ 0Z(3), 0Z(3), 0Z(3), 0Z(3), Z(3), 0Z(3) ],
[ 0
Z(3), 0Z(3), 0Z(3), Z(3)^0, 0Z(3), 0Z(3) ],
[ 0Z(3), 0Z(3), Z(3)^0, 0Z(3), 0Z(3), 0Z(3) ],
[ 0
Z(3), Z(3), 0Z(3), 0Z(3), 0Z(3), 0Z(3) ],
[ Z(3), 0Z(3), 0Z(3), 0Z(3), 0Z(3), 0Z(3) ] ]
gap> form := BilinearFormByMatrix(gram,f);
< bilinear form >
gap> BaseChangeToCanonical(form);
[ [ Z(3)^0, 0
Z(3), Z(3)^0, Z(3)^0, 0Z(3), Z(3)^0 ],
[ Z(3)^0, 0
Z(3), Z(3), Z(3), 0Z(3), Z(3)^0 ],
[ Z(3)^0, Z(3), Z(3), Z(3)^0, 0
Z(3), Z(3) ],
[ Z(3), 0Z(3), Z(3)^0, Z(3), Z(3), Z(3)^0 ],
[ Z(3), 0
Z(3), Z(3)^0, Z(3), 0Z(3), Z(3)^0 ],
[ 0
Z(3), Z(3)^0, Z(3), Z(3)^0, Z(3), 0*Z(3) ] ]
gap> Display(form);
Hyperbolic bilinear form
Gram Matrix:
. . . . . 2
. . . . 2 .
. . . 1 . .
. . 1 . . .
. 2 . . . .
2 . . . . .
Witt Index: 3
gap> TypeOfForm(form);
1

As I mentioned, the generate_tst_files.g was meant to convert the .g file into a piece of xml to be included in the xml documentation file. Just to avoid to manually have to copy/paste GAP output into all these example files. So I understood that tst files can be generated from the GAP output present in the documentation files. But first I want to convert the .g files into files containing the output.

It can be seen example what I mean in the forms/examples directory. There are the directories 'gap', 'output', and 'include'. The 'gap' directory contains 48 .g files, just the files containing the GAP commands making an example. The 'output' directory contains that GAP output, the 'include' directory contains the .xml files ready to be included in the documentation.

@ThomasBreuer
Copy link
Contributor

My interpretation of your setup is that you do not care about the format of the GAP output, you just plug in whatever GAP returns.
I would propose a setup in which the GAP output belongs to the source data (under version control), because then I have a chance to notice whether the output has changed in the latest GAP version. Sometimes such changes are a reason to reformulate some text about the example or to rearrange the example itself.

In order to get an initial version of the manual examples with input and output inside the *.xml files, I would replace the <#Include SYSTEM "../examples/include/something.include"> statements by the contents of the corresponding files ../examples/include/something.include. (I am not aware of a special tool for this task.)
Afterwards the examples directory is not needed anymore, and Test and RunExamples can be used to adjust the manual examples automatically to the current GAP version. This yields an easy way to compare the old GAP output with the new output --ideally there are no differences, but if there are differences then they are interesting.

(One more suggestion for a simplification: If the examples are enclosed by <Example><![CDATA[...]]></Example> instead of <Example>...</Example> then the text need not be escaped, in particular < characters may occur inside the example.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants