Unittest sample : Proposal for issue #4 #57

vinklein · 2018-01-17T14:16:26Z

This pull request port some GP doctests as Python unittest.
This a solution proposal for issue #4.
A more complete but not fully working version can be found here unittest_dev.

What is your advices on this implementation ?
Comments and critics are welcome

fix setUp and tearDown in gamma.py

'list', 'log', 'mathnf', 'contfrac', 'minim', 'minmax','modfun','modular', 'nfrootsof1', 'nfsplitting', 'norm'

prec, prime, primes, qfb, qfbclassno, qfsolve, quadray

'rootsreal', 'ser', 'subst','sumdedekind', 'sumformal', 'zeta'

add precision to factorial call

are installed.

bestappr ellanal idealramgroups quadray polred

polylog gamma nfsplitting rootsreal ser subst

Test working with 2.9.3 2.8.1.beta and snapshot versions

videlec · 2018-01-17T14:28:19Z

What is the file unittest_sample used for?

vinklein · 2018-01-17T14:34:31Z

unittest_sample is this branch, if you mean unittest_template, it's a unused file i shouldn't have commit : )

videlec

I like the usage of unittest very much. Though I think that you should try harder to not compare strings but actual objects. They can not do that in PARI/GP but here we do.

videlec · 2018-01-17T14:31:59Z

tests/unittests/alltest.py

+galpol_installed = True
+seadata_installed = True
+
+# test extensions presence.


I would propose to encapsulate these in functions has_seadata, has_galpol, has_elldata, etc that would actually be part of the cypari2 library. Perhaps as methods of Pari.

videlec · 2018-01-17T14:34:37Z

tests/unittests/alltest.py

+
+
+for t in testmodules:
+    if not ((t in require_galpol and not galpol_installed) or (t in require_seadata and not seadata_installed)):


please change the condition to

(galpol_installed or t not in require_galpol) and (seadata_installed or t not in require_seadata)

videlec · 2018-01-17T14:37:16Z

tests/unittests/alltest.py

+
+for t in testmodules:
+    if not ((t in require_galpol and not galpol_installed) or (t in require_seadata and not seadata_installed)):
+        try:


Couldn't this be uniform among all the files? This try/except is a bit ugly.

videlec · 2018-01-17T14:39:42Z

tests/unittests/alltest.py

@@ -0,0 +1,87 @@
+import unittest


You should briefly document the mechanic of the tests here.

videlec · 2018-01-17T14:40:09Z

tests/unittests/bern.py

+
+class TestBern(unittest.TestCase):
+    def test_bern(self):
+        pari.bernfrac(0);


what this is supposed to test? and why keeping ;?

It test that it doesn't crash. I keep them mainly to stick with the original GP test file.

I keep ; during developement as an help to associate results with the corresponding test.
Keeping them after can has no sense. I will remove them.

videlec · 2018-01-17T14:40:16Z

tests/unittests/bern.py

+class TestBern(unittest.TestCase):
+    def test_bern(self):
+        pari.bernfrac(0);
+        pari.bernfrac(1);


jdemeyer · 2018-01-17T14:43:37Z

I think that we should not add all those file to cypari2, especially because we want to support multiple versions of PARI and the version that we add is just one version.

Instead we should just use tests_generator.py (where is that script anyway?) and test directly the output of that.

videlec · 2018-01-17T14:59:53Z

I don't think that test_generator.py is actually generating the test files in their current form. This message is very misleading. Though it is important to keep the information from which PARI test it came from.

vinklein · 2018-01-17T15:04:14Z

The comments # This file was generated by tests_generator.py should be ignored for now (And removed).
test_generator.py exist but it only partly generate the unittest files, most of the work has been done manually.
That's because the original gp doctests are written with a lot of different kind of syntax.

jdemeyer · 2018-01-17T22:53:17Z

Sorry for sounding a bit negative, but I don't quite understand why you are using unittests here. I thought that the main point of #4 was to have Python doctests. That is not what you doing here.

Now it seems that you just copy PARI tests to cypari2 which is a bit pointless because it's not the job of cypari2 to test PARI. It should be the job of cypari2 to test the wrapping mechanism, not to test the PARI library itself.

vinklein · 2018-01-18T08:57:40Z

Sorry for sounding a bit negative, but I don't quite understand why you are using unittests here. I thought that the main point of #4 was to have Python doctests. That is not what you doing here.

An understandable question. If the main goal is to have Python doctest this PR have no sense. But the choice of unittests is more an answer to the question "What is the good way to port GP tests into cypari2".
For the choice unittest vs doctest :

unittest is more flexible overall.
IMO the main question when chosing between unittest or doctest is to know if your tests are mainly here for informative value (showing what can be done) or if your tests are here to try to test every case.
GP tests looks more to be of the latter kind, and a lot of tests result are pretty unreadable given their size or structure (like testing many function with many parameters in nested loops) and therefore have a poor value as documentation.

It should be the job of cypari2 to test the wrapping mechanism, not to test the PARI library itself.

Can you expand on that ?
Does test something like pari.addprimes([pari.nextprime(1e9), pari.nextprime(1e10)]) has no added value for you ?

videlec · 2018-01-18T09:09:37Z

The GP tests are a natural source of code examples (that is moreover tested). This PR is not testing PARI/GP in any way but rather if GP code can easily be ported in cypari2. It checks for coherences and helps identifying what is the missing in cypari2 vs GP.

Add file's description. Remove the option to use test suite.

jdemeyer · 2018-01-18T12:51:44Z

IMO the main question when chosing betwwen unittest or doctest is to know if your tests are mainly here for informative value (showing what can be done)

The GP doctests (i.e. the tests appearing in the PARI user documentation) are meant as documentation/examples, analogous to Python doctests.

The PARI unittests (i.e. the tests in the src/pari directory in the PARI sources) are meant as pure tests.

I think it makes most sense to port the GP doctests to cypari2. Porting the PARI unittests seems useless.

vinklein · 2018-01-31T09:32:27Z

Porting the PARI unittests seems useless.

I really don't see how it can be useless. Unless you have a way to ensure that cypari2 is perfectly iso-functional with pari. Saying it's useless it's the same as saying "the wrapping is perfect and will never fail".
I thinks you mean it's not worth it.

Now what do we do with these unittest ?

jdemeyer · 2018-01-31T09:44:03Z

the wrapping is perfect and will never fail

It's not that the wrapping will never fail. It's more like "the wrapping will either work or fail badly". It would be really surprising if the wrapping would work for most functions but not all.

Of course, the cases where we manually wrap some functions should be tested properly.

Vincent Klein added 25 commits January 17, 2018 14:34

Initial tests units

eabf0a7

functions utils for test units

e3f02be

Add test of linear group

538826a

fix setUp and tearDown in gamma.py

Add tests for several files

090cd5e

'list', 'log', 'mathnf', 'contfrac', 'minim', 'minmax','modfun','modular', 'nfrootsof1', 'nfsplitting', 'norm'

add number and pol testunits

cfe2c5b

Add tests for several files:

6d9c070

prec, prime, primes, qfb, qfbclassno, qfsolve, quadray

corrections de test

cda8f0f

Add tests for several files:

33e4a12

'rootsreal', 'ser', 'subst','sumdedekind', 'sumformal', 'zeta'

isprime : use ** operator instead of int(pow(x))

c7d055f

unittest polred

ade1148

unittest Add polred file in alltest

60efa3a

unittest : add bit test file

5692909

unittest : add charpoly test file

4c542b5

Unittest replace call precision=127 by precision=128

3e0d9b3

number.py

7104813

add precision to factorial call

add unittest to the 'make check' command

572a442

alltest.py set return code to 1 if faillures in test suite

f097e44

alltest.py run a test unit only if his required extension

471bdd5

are installed.

alltest.py run a test unit only if his required extension

677b3d3

are installed.

To squash

e50d5d1

testutils : xrange compatibility for python3

be420a8

unittests : Compatibility with python3

19dc453

unittests : remove tests not working with snapshots version of gap

54dcdf4

bestappr ellanal idealramgroups quadray polred

unittests : remove tests not working with 2.8.1.beta version of gap

11024a2

polylog gamma nfsplitting rootsreal ser subst

unittests : Build a sample branch with only fully working tests

c5eae74

Test working with 2.9.3 2.8.1.beta and snapshot versions

unittests : remove unittest_template file

a3fd917

videlec reviewed Jan 17, 2018

View reviewed changes

Vincent Klein added 2 commits January 18, 2018 11:48

unittests : Enhance alltest.py

f2554cf

Add file's description. Remove the option to use test suite.

unittests : Remove comments from tests_generator.py

3505e1d

vinklein closed this Apr 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unittest sample : Proposal for issue #4 #57

Unittest sample : Proposal for issue #4 #57

vinklein commented Jan 17, 2018

videlec commented Jan 17, 2018

vinklein commented Jan 17, 2018

videlec left a comment

videlec Jan 17, 2018

videlec Jan 17, 2018

videlec Jan 17, 2018

videlec Jan 17, 2018

videlec Jan 17, 2018

vinklein Jan 18, 2018

videlec Jan 17, 2018

jdemeyer commented Jan 17, 2018

videlec commented Jan 17, 2018

vinklein commented Jan 17, 2018 •

edited

Loading

jdemeyer commented Jan 17, 2018

vinklein commented Jan 18, 2018 •

edited

Loading

videlec commented Jan 18, 2018

jdemeyer commented Jan 18, 2018

vinklein commented Jan 31, 2018 •

edited

Loading

jdemeyer commented Jan 31, 2018



		for t in testmodules:
		if not ((t in require_galpol and not galpol_installed) or (t in require_seadata and not seadata_installed)):

Unittest sample : Proposal for issue #4 #57

Unittest sample : Proposal for issue #4 #57

Conversation

vinklein commented Jan 17, 2018

videlec commented Jan 17, 2018

vinklein commented Jan 17, 2018

videlec left a comment

Choose a reason for hiding this comment

videlec Jan 17, 2018

Choose a reason for hiding this comment

videlec Jan 17, 2018

Choose a reason for hiding this comment

videlec Jan 17, 2018

Choose a reason for hiding this comment

videlec Jan 17, 2018

Choose a reason for hiding this comment

videlec Jan 17, 2018

Choose a reason for hiding this comment

vinklein Jan 18, 2018

Choose a reason for hiding this comment

videlec Jan 17, 2018

Choose a reason for hiding this comment

jdemeyer commented Jan 17, 2018

videlec commented Jan 17, 2018

vinklein commented Jan 17, 2018 • edited Loading

jdemeyer commented Jan 17, 2018

vinklein commented Jan 18, 2018 • edited Loading

videlec commented Jan 18, 2018

jdemeyer commented Jan 18, 2018

vinklein commented Jan 31, 2018 • edited Loading

jdemeyer commented Jan 31, 2018

vinklein commented Jan 17, 2018 •

edited

Loading

vinklein commented Jan 18, 2018 •

edited

Loading

vinklein commented Jan 31, 2018 •

edited

Loading