Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#6343 closed enhancement (fixed)

[with patch, positive review] Adds TestSuite(object).run() generic testing framework

Reported by: nthiery Owned by: nthiery
Priority: major Milestone: sage-4.1.2
Component: doctest coverage Keywords: testunit
Cc: sage-combinat, cwitty, roed, saliola, mvngu Merged in: Sage 4.1.2.alpha2
Authors: Nicolas M. Thiéry Reviewers: Mike Hansen, Minh Van Nguyen
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by nthiery)

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

Attachments (3)

trac_6343-sage_object-test-nt.patch (14.2 KB) - added by nthiery 8 years ago.
trac_6343-reviewer.patch (4.5 KB) - added by mvngu 8 years ago.
reviewer patch; fixes typos and docstring formatting
trac_6343-reviewer-nt.patch (1.7 KB) - added by nthiery 8 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 8 years ago by nthiery

  • Description modified (diff)

comment:2 Changed 8 years ago by was

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

William

comment:3 Changed 8 years ago by nthiery

  • Status changed from new to assigned
  • Summary changed from Adds SageObject.check() generic testing framework to [with patch, needs review] Adds SageObject.check() generic testing framework

comment:4 Changed 8 years ago by was

  • Summary changed from [with patch, needs review] Adds SageObject.check() generic testing framework to [with patch, needs work] Adds SageObject.check() generic testing framework

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

comment:5 Changed 8 years ago by nthiery

  • Description modified (diff)
  • Keywords testunit added
  • Summary changed from [with patch, needs work] Adds SageObject.check() generic testing framework to [with patch, needs review] Adds TestSuite(object).run() generic testing framework

comment:6 Changed 8 years ago by nthiery

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).

comment:7 Changed 8 years ago by nthiery

  • Description modified (diff)

comment:8 Changed 8 years ago by was

I like the new design and class/function names.

Changed 8 years ago by nthiery

comment:9 Changed 8 years ago by nthiery

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

comment:10 Changed 8 years ago by mhansen

  • Authors changed from nthiery to Nicolas Thiery
  • Reviewers set to Mike Hansen
  • Summary changed from [with patch, needs review] Adds TestSuite(object).run() generic testing framework to [with patch, positive review] Adds TestSuite(object).run() generic testing framework

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

comment:11 Changed 8 years ago by nthiery

  • Authors changed from Nicolas Thiery to Nicolas M. Thiéry

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

comment:12 Changed 8 years ago by mhansen

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

Changed 8 years ago by mvngu

reviewer patch; fixes typos and docstring formatting

comment:13 follow-up: Changed 8 years ago by mvngu

  • Reviewers changed from Mike Hansen to Mike Hansen, Minh Van Nguyen
  • Summary changed from [with patch, positive review] Adds TestSuite(object).run() generic testing framework to [with patch, needs work] Adds TestSuite(object).run() generic testing framework

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.

Changed 8 years ago by nthiery

comment:14 in reply to: ↑ 13 Changed 8 years ago by nthiery

Replying to 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.

comment:15 Changed 8 years ago by nthiery

  • Summary changed from [with patch, needs work] Adds TestSuite(object).run() generic testing framework to [with patch, needs review] Adds TestSuite(object).run() generic testing framework

comment:16 Changed 8 years ago by nthiery

  • Cc mvngu added

comment:17 Changed 8 years ago by mhansen

  • Summary changed from [with patch, needs review] Adds TestSuite(object).run() generic testing framework to [with patch, positive review] Adds TestSuite(object).run() generic testing framework

Everything looks good to me now. Apply all patches.

comment:18 Changed 8 years ago by mvngu

  • Merged in set to Sage 4.1.2.alpha2
  • Resolution set to fixed
  • Status changed from assigned to closed

Merged patches in this order:

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

comment:19 Changed 8 years ago by nthiery

Thanks Mike, thanks Minh!

Note: See TracTickets for help on using tickets.