Skip to content

Conversation

@olexandr-konovalov
Copy link
Member

This implements the idea from issue #172 to make ReadTest obsolete and replace it by a call to test with comparison up to whitespaces. I've tested it on those packages which are still using ReadTests and it works fine - the tests now produce the new output, and nothing is broken.

Probably the kernel function READ_TEST_OR_LOOP could be also renamed to READ_LOOP.

@olexandr-konovalov
Copy link
Member Author

Ignore the last sentence - there is already READ_LOOP calling READ_TEST_OR_LOOP and that is the single place where READ_TEST_OR_LOOP is called. Maybe the latter should just me incorporated into the former, or just renamed in some other way.

@ChrisJefferson
Copy link
Contributor

I'm happy to see this merged -- it might well break some package's test but if it does the fixes will be very simple, so it seem like a good candidate for the 4.8 beta.

@olexandr-konovalov
Copy link
Member Author

@ChrisJefferson thanks. As I've said, I've run "make testpackages" and because the comparison is up to whitespaces, I don't think that any reported diffs were new - they were just reported in a better way. I am going to merge this then.

olexandr-konovalov pushed a commit that referenced this pull request Oct 15, 2015
Replaced ReadTest by Test with comparison up to whitespaces.
@olexandr-konovalov olexandr-konovalov merged commit 3104ea1 into gap-system:master Oct 15, 2015
@olexandr-konovalov
Copy link
Member Author

I've merged this. One extra reason for this is that now outputs of default tests in packages will be more uniform and it will be easy to detect whether they have diffs or not in automated scenarios and send failure notifications to package maintainers.

@olexandr-konovalov
Copy link
Member Author

P.S. Just discovered that ReadTest does not complain about non-existing file:

gap> ReadTest("zzz");
false

(it's not documented what it returns). It happened that for a couple of packages using ReadTest the path to the test file was not set correctly, so their tests were not run automatically. That was revealed after this PR, since Test triggers an error in this case:

gap> Test("zzz");
Error, Cannot read file zzz
 called from
<function "Test">( <arguments> )
 called from read-eval loop at line 2 of *stdin*
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue
brk> 

@olexandr-konovalov olexandr-konovalov deleted the obsoleting-readtest branch November 30, 2015 15:03
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.

2 participants