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

Adds TestSuite(object).run() generic testing framework #6343

Closed
nthiery opened this issue Jun 16, 2009 · 23 comments
Closed

Adds TestSuite(object).run() generic testing framework #6343

nthiery opened this issue Jun 16, 2009 · 23 comments

Comments

@nthiery
Copy link
Contributor

nthiery commented Jun 16, 2009

This patch implements TestSuite(object).run() which runs
systematic checks on the object. Here is a typical call:

     sage: TestSuite(ZZ).run(verbose = True)
     running ._test_an_element() ... done
     running ._test_element_pickling() ... done
     running ._test_not_implemented_methods() ... done
     running ._test_pickling() ... done

In practice, TestSuite(o).run() runs all the methods named test* of the object o.
The test* methods are typically implemented by abstract super classes
and in particular via categories, in order to enforce standard
behavior and API (_test_pickling, _test_an_element), or provide
mathematical sanity checks (_test_associativity).

For consistent error reporting, the test* methods in turn must use
the new gadget sage.misc.sage_unittest.InstanceTester to actually
run the tests.

This is used by the category patches #5891 and followers

CC: @sagetrac-sage-combinat @sagetrac-cwitty @roed314 @saliola @sagetrac-mvngu

Component: doctest coverage

Keywords: testunit

Author: Nicolas M. Thiéry

Reviewer: Mike Hansen, Minh Van Nguyen

Merged: Sage 4.1.2.alpha2

Issue created by migration from https://trac.sagemath.org/ticket/6343

@nthiery nthiery added this to the sage-4.1.2 milestone Jun 16, 2009
@nthiery nthiery self-assigned this Jun 16, 2009
@nthiery

This comment has been minimized.

@williamstein
Copy link
Contributor

comment:2

Change the name from obj.check() to obj._check(). It is not reasonable that if one does obj. on any Sage object, one sees check.

William

@williamstein
Copy link
Contributor

comment:4

Change the name from obj.check() to obj._check(). It is not reasonable that if one does obj. on any Sage object, one sees check.

@nthiery
Copy link
Contributor Author

nthiery commented Jun 26, 2009

Changed keywords from none to testunit

@nthiery

This comment has been minimized.

@nthiery nthiery changed the title Adds SageObject.check() generic testing framework Adds TestSuite(object).run() generic testing framework Jun 26, 2009
@nthiery
Copy link
Contributor Author

nthiery commented Jun 26, 2009

comment:6

Patch reworked, after the discussion on sage-devel.

Note: After peeking again into the testunit framework, I finally went for TestSuite(object).run() which
is most consistent with it, while being meaningful.

The otherwise discussed functionality TestSuite(object).associativity() will be implemented in a later patch
(if deemed useful in the meantime).

@nthiery

This comment has been minimized.

@williamstein
Copy link
Contributor

comment:8

I like the new design and class/function names.

@nthiery
Copy link
Contributor Author

nthiery commented Jul 10, 2009

comment:9

Attachment: trac_6343-sage_object-test-nt.patch.gz

Oops, the doctests were broken (I had forgotten to rename stuff in there)

@mwhansen
Copy link
Contributor

Reviewer: Mike Hansen

@mwhansen
Copy link
Contributor

comment:10

Looks good to me. We just have to make sure to re-enable the missing tests when the category stuff is added.

@mwhansen
Copy link
Contributor

Changed author from nthiery to Nicolas Thiery

@nthiery
Copy link
Contributor Author

nthiery commented Jul 16, 2009

Changed author from Nicolas Thiery to Nicolas M. Thiéry

@nthiery
Copy link
Contributor Author

nthiery commented Jul 16, 2009

comment:11

Thanks for the review!
I just fixed my name to please my grand father :-)

@mwhansen
Copy link
Contributor

comment:12

No worries. I really should just add key tomy keyboard layout :-)

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jul 16, 2009

reviewer patch; fixes typos and docstring formatting

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jul 16, 2009

Changed reviewer from Mike Hansen to Mike Hansen, Minh Van Nguyen

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jul 16, 2009

comment:13

Attachment: trac_6343-reviewer.patch.gz

The patch trac_6343-reviewer.patch fixes some typos and copy the docstring for __init__() in the class InstanceTester to the class itself. This is necessary as docstring in methods whose names begin with an underscore (i.e. _) would not show up in the reference manual.

Apart from that, here's a small issue. Can you provide examples and/or tests for the function _test_pickling() in sage/structure/sage_object.pyx? The doctest coverage for sage/structure/sage_object.pyx is very short of 50% and we don't want to make it more difficult to reach 100% coverage.

@nthiery
Copy link
Contributor Author

nthiery commented Jul 20, 2009

comment:14

Attachment: trac_6343-reviewer-nt.patch.gz

Replying to @sagetrac-mvngu:

The patch trac_6343-reviewer.patch fixes some typos and copy the docstring for __init__() in the class InstanceTester to the class itself. This is necessary as docstring in methods whose names begin with an underscore (i.e. _) would not show up in the reference manual.

Agreed, that's better (but not for that argument: my opinion is that _* methods, or at least * and * methods should show up in the manual; but that's another discussion).

Thanks for your doc fixes! I double checked them.

Apart from that, here's a small issue. Can you provide examples and/or tests for the function _test_pickling() in sage/structure/sage_object.pyx? The doctest coverage for sage/structure/sage_object.pyx is very short of 50% and we don't want to make it more difficult to reach 100% coverage.

Thanks also for spotting the missing doctest. The attached patch fixes this.

@mwhansen
Copy link
Contributor

mwhansen commented Sep 8, 2009

comment:17

Everything looks good to me now. Apply all patches.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Sep 8, 2009

Merged: Sage 4.1.2.alpha2

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Sep 8, 2009

comment:18

Merged patches in this order:

  1. trac_6343-sage_object-test-nt.patch
  2. trac_6343-reviewer.patch
  3. trac_6343-reviewer-nt.patch

@sagetrac-mvngu sagetrac-mvngu mannequin removed the s: positive review label Sep 8, 2009
@sagetrac-mvngu sagetrac-mvngu mannequin closed this as completed Sep 8, 2009
@nthiery
Copy link
Contributor Author

nthiery commented Sep 8, 2009

comment:19

Thanks Mike, thanks Minh!

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

3 participants