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

minimal automated test for XGAP #4

Open
olexandr-konovalov opened this issue Nov 17, 2016 · 15 comments
Open

minimal automated test for XGAP #4

olexandr-konovalov opened this issue Nov 17, 2016 · 15 comments

Comments

@olexandr-konovalov
Copy link
Member

Comes from gap-system/gap#946 by @frankluebeck.

I can compile XGAP on my Ubuntu machine, and it normally compiles on Jenkins slaves. But of course because one can not load it with LoadPackage, there is no automated check for this.

Could this work better? For example, a test file (which is referenced in PackageInfo.g in the component TestFile) which just checks e.g. with IsExistingFile that the binary exists? Of course, that binary may not work, and may be a relic from an old build (unless like in Jenkins, each build starts from a clean workspace), so that is not a guarantee. Any better ideas? Special testing mode to start binary so that it prints some test message and quits? Something like

xgap --test
This is XGAP. It seems to work. You need to start it with xgap.sh. Bye!
@fingolfin
Copy link
Member

I don't understand in how far this "comes from" Frank's issue. Nor do I know what you mean by "Could this work better?". What is "this"? What is it you want to work better?

From what you write, I am guessing that you want to have some kind of test "whether xgap was compiled successfully". Of course one could implement such a test, but where should that test be run from? That in turn likely depends on which problem exactly you are trying to get fixed here - which is unclear to me.

@olexandr-konovalov
Copy link
Member Author

olexandr-konovalov commented Nov 17, 2016

I admit that I sometimes assume that people can read my mind, especially when I'm writing things in a hurry before leaving the office. On the other hand, I was intended for you, and I was confident that your intuition will help you to read my mind, and you actually did this pretty well ;)

Anyhow, to explain that to future readers who may be not package authors and not familiar with GAP regression tests:

  1. The problem is I don't have much to say in response to @frankluebeck's report about XGAP not compiling. I know that it "usually" works, e.g. it compiles on my desktop with sufficiently many dependencies installed. I can also inspect build logs in jenkins periodically to find out if it builds on other Jenkins slaves. But because there are no automated tests for XGAP, I mostly rely on package authors' own testing in their release workflow for XGAP.
  2. To solve this, I suggest to establish a test that will run automatically as a part of the GAP release workflow. This test will be invoked by make testpackages. This target runs the test file which is either a GAP input file or a GAP test file in .tst format.

@frankluebeck
Copy link
Member

@alex-konovalov : You misinterpreted my comment in gap-system/gap#946. I hope it becomes clearer from my second comment there.

@olexandr-konovalov
Copy link
Member Author

olexandr-konovalov commented Nov 18, 2016

@frankluebeck you wrote

(Unfortunately, xgap does not compile on my system and BuildPackages.sh does not give any useful hint how to fix this.

so it was hard to understand it differently - thanks for further clarifications. I think my suggestion may remain open, though with slightly lower priority then.

@RussWoodroofe
Copy link
Contributor

If there were interest in starting xgap.sh with a test file, I suggest that some basic commands to include could be the following. Obviously, it's trickier to check that a GraphicSheet looks right, but this at least checks that basic functionality is there and error-free.

"Hello"; # check that TermEncoding is set correctly
L:=GraphicSubgroupLattice(SymmetricGroup(4)); # open a graphic sheet
Close(L); # close the sheet
quit;

@fingolfin
Copy link
Member

My problem with this is that in order to run tests, one needs an X-Window server running, and none of our test rigs has one, as far as I know; perhaps I could install one on Travis, but even then, it seems hard to come up with a good way to do a meaningful automated test that goes beyond "does it compiler". Not saying it is impossible (of course it is possible), but rather that I don't know how to do it, so I'd have to invest considerable effort to figure it out -- and all that to get testing done for a piece of software that in my eyes is mostly dead anyway.

tl;dr: I have absolutely no plans to ever implement this myself, but I'll welcome patches for it.

@olexandr-konovalov
Copy link
Member Author

@fingolfin very much agree with this.

@RussWoodroofe
Copy link
Contributor

I added looking at this to my "possibly do" list for the next time I'm in "programming mode".

@RussWoodroofe
Copy link
Contributor

RussWoodroofe commented Mar 4, 2020

I've given this some thought, though I still haven't had enough programming time to implement. But I think a good approach would be to have a minimal shell that runs "gap -p", does some minimal interpretation of the input/output, and otherwise is a text mode program. That wouldn't test actual xgap functionality, but would test interaction of other parts of GAP and other packages with xgap. The last is where most of the problems have occurred in the last years.

I'm still interested in doing something with this, and now that I have an idea of how it should go, I think I can put something together in the not-too-distant future.

@RussWoodroofe
Copy link
Contributor

Well, it's been almost a year, but the current GAP Days has inspired me to work on this. The attached GAP .g file starts an extra instance of GAP via InputOutputLocalProcess, starting with the "-p" (package mode) flag. If the XGAP library is available, it will automatically be loaded. The code as it stands now will issue a "Print("Hello world");" command and then exit. (Of course, the Hello World command could very easily be replaced with a command to load an auxiliary test file, or with a sequence of commands, or whatever.)

Technically, this basically undoes the @ encoding introduced by package mode. It also listens for start-of-input or error, and responds to the XGAP @w commands that query for colors and font-size. (The responses are hard-coded, and match the responses given by Gap.app to the same queries. I think plain XGAP generally responds a little different to the font queries, but also am fairly confident that it doesn't make any difference.)

I'd suggest that, for such an auxiliary test file, it would be a good start to test some simple text-based IO and to try a LoadAllPackages(). (That at least would catch the two recent conflicts with XGAP.)

It'd also be possible to issue a few simple XGAP commands in a test file, and hard-code responses in this .g file (similarly to the color and font queries). But I don't see much value in doing so at this time.

I've attached the file here for the moment. Alex, Max, this is a bit updated over what I sent you by email (should be a little smarter in finding GAP). I can try and put into a pull request if that is so indicated.
xgap_test.g.zip

@olexandr-konovalov
Copy link
Member Author

@RussWoodroofe I think making this a PR and trying to configure GH Actions to run it would be the best. Thanks!

@RussWoodroofe
Copy link
Contributor

Added PR for the testing framework (this file).

As far as implementing actual tests: Do I understand correctly that setting up so TestPackage("xgap") works will automatically make it work in GH Actions? Assuming so, would it be problematic to try to run LoadAllPackages()?

@fingolfin
Copy link
Member

No, GH actions need to be set up first (which I just did in PR #19), after merging your PR -- thanks for that, BTW! And sorry for only merging I simply had overlooked and/or forgotten about it :-/.

@fingolfin
Copy link
Member

As far as I can tell, the tests @RussWoodroofe added test the GAP "window" support, but not xgap specifically, right? That would make it somewhat orthogonal to the original intent of this issue, which asks for a test that determines at least if xgap was successfully compiled, which I think we still don't have, do we?

As such... perhaps the new tests should instead be part of GAP resp. the GAP test suite?

@RussWoodroofe
Copy link
Contributor

Thank you for finishing this off! I was unsure of the last steps that were needed, and was going to bring it up at GAPDays. Yes, these tests mainly test GAP's package -p mode; there are some interactions with the xgap library (which activates under package mode). They would catch the last couple of places where xgap broke, which involved subtle interactions between package mode, xgap, and GAPDoc.

It would be possible to add tests for the xgap library (the .g files) by feeding GAP canned @ commands and checking the responses. I'd be potentially willing to do this. Testing the xgap C executable is a harder problem, which I don't immediately see a good solution for.

I don't have a strong opinion on where the tests belong. Since there is potential for further developing them to test the xgap library, though, perhaps it would make sense to keep them under xgap for the time being?

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

No branches or pull requests

4 participants