Update doctesting framework

[robertwb]

There are several improvements that would be good to make, including but not limited to:

1. Forking rather than starting up a new Sage each time.
2. Customizable comparison rather than mutating doctests into new doctests (e.g. followup to better numerical accuracy testing #10952).
3. Eliminate need to write temporary files for every doctested file.
4. Constant headaches caused by the difference in doctesting library vs. non-library files.
5. The ability to drop into a debugger if a doctest raises an exception.

Robert's code is at

http://code.google.com/p/sagemath-timer/

Apply attachment: 12415_framework.patch, attachment: 12415_doctest_fixes.patch, attachment: 12415_doc.patch, attachment: 12415_review.patch, attachment: 12415_test.patch, attachment: 12415_review_review.patch, attachment: 12415_review3.patch, attachment: 12415_manifest.patch, attachment: 12415_rebase_58.patch

Apply attachment: 12415_script.patch and attachment: 12415_script_review.patch to the scripts repo

Apply attachment: 12415_spkg_bin_sage.patch to the root repo

For follow-up or other doctest-related tickets see #11337.

Depends on #13147
Depends on #13146
Depends on #13145
Depends on #12723
Depends on #12392
Depends on #12393
Depends on #12395
Depends on #12396
Depends on #12397
Depends on #12381
Depends on #12382
Depends on #12383
Depends on #12384
Depends on #11871
Depends on #13195
Depends on #13121
Depends on #13748
Depends on #13899
Depends on #12719
Depends on #5155
Depends on #14070
Depends on #14079
Depends on #14150
Depends on #14158
Depends on #14182
Depends on #14184
Depends on #14054
Depends on #14063
Depends on #13605
Depends on #14111
Depends on #14254
Depends on #14242
Depends on #14253

CC: @kini @ohanar @jhpalmieri

Component: doctest framework

Author: David Roe, Robert Bradshaw, Jeroen Demeyer

Reviewer: Jeroen Demeyer, David Roe

Merged: sage-5.9.beta0

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

[roed314]

comment:3

I'm working on this. E-mail me if you want to collaborate.

[roed314]

Dependencies: #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384

[roed314]

Changed dependencies from #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384 to #12723, #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384

[robertwb]

comment:9

12415_2.patch seems to contain a lot of tempfile changes, could you split out the doctest changes into a separate patch (or are these affecting doctests?)

[ohanar]

comment:11

apparently even I forget my trac username is not what it should be

[roed314]

Changed dependencies from #12723, #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384 to #13145, #12723, #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384

[roed314]

Changed dependencies from #13145, #12723, #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384 to #13146, #13145, #12723, #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384

[roed314]

Changed dependencies from #13146, #13145, #12723, #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384 to #13147,#13146, #13145, #12723, #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384

[roed314]

comment:15

I've moved the patches dealing with temporary files to #13147.

[roed314]

comment:19

Ready for review!

There are still a few known issues:

• valgrinding under sage -t doesn't work. I haven't used valgrind much and don't know how to resolve this.
• The fix at Delay the evaluation of SAGE_TMP in order to facilitate forking in doctesting #13147 probably needs work: I'm getting a failure in sage.rings.polynomial.multi_polynomial_libsingular to do with garbage collection. I'm working on it.
• I'm in the process of updating the developer guide.

But I wanted to get some other people involved in reviewing.

[jhpalmieri]

comment:20

This may be a silly question, but how do I use the new framework? Don't we need changes to the scripts repo, too? (If I just run sage -t FILES, I don't see the various messages, like Doctesting ..., that should be printed by run_doctests are not printed, so sage -t is not yet using the new framework.)

One of the patches also didn't apply cleanly to 5.1.beta5:

hg qimport -P ~/Downloads/12415_stderr.patch 
adding 12415_stderr.patch to series file
applying 12415_stderr.patch
patching file sage/lfunctions/lcalc.py
Hunk #1 FAILED at 226
1 out of 2 hunks FAILED -- saving rejects to file sage/lfunctions/lcalc.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh 12415_stderr.patch

[roed314]

comment:21

Did you apply the new dependencies (#13145, #13146, #13147)? There are some more patches to other repositories: I'll update the ticket description.

[roed314]

comment:23

The description is updated: let me know if you have any more problems. I think you need to apply 12415_stderr_vs_51b5.patch instead of 12415_stderr.patch.

[jhpalmieri]

comment:24

Regarding attachment: 12415_spkg_bin_sage.patch, the file in question should be part of the root repo.

[jhpalmieri]

comment:25

One regression: with the old set-up, running sage -tp FILES (with no argument for tp) would choose the number of threads automatically. With the new set-up, this command gives an error, because FILES is not an integer. Therefore I think that, for example, make ptestlong will fail. (The file sage-runtests says that the default value for nthreads is 1, but I don't think optparse is dealing with the missing argument properly here. Also, passing the argument 0 causes doctests to never happen: maybe it's using 0 threads instead of minimum(8, cpu_count()).

[jhpalmieri]

comment:26

I would suggest the following two changes: for the scripts repo:

diff --git a/sage-runtests b/sage-runtests
--- a/sage-runtests
+++ b/sage-runtests
@@ -5,7 +5,7 @@ import optparse, os, sys
 if __name__ == "__main__":
     parser = optparse.OptionParser()
 
-    parser.add_option("-p", "--nthreads", type=int, default=1, metavar="N", help="tests in parallel using N threads with 0 interpreted as minimum(8, cpu_count())")
+    parser.add_option("-p", "--nthreads", type="string", default=1, metavar="N", help="tests in parallel using N threads with 0 interpreted as minimum(8, cpu_count())")
     parser.add_option("--serial", action="store_true", default=False, help="run tests in a single process in series")
     parser.add_option("--timeout", type=int, default=-1, help="timeout (in seconds) for doctesting one file")
     parser.add_option("-a", "--all", action="store_true", default=False, help="test all files in the Sage library")
@@ -49,7 +49,20 @@ if __name__ == "__main__":
     parser.add_option("--stats_path", "--stats-path", default=os.path.join(os.path.expanduser("~/.sage/timings2.json")), \
                           help="path to a json dictionary for the latest run storing a timing for each file")
 
+    options, args = parser.parse_args()
+    try:
+        options.nthreads = int(options.nthreads)
+    except ValueError:
+        args.insert(0, options.nthreads.strip())
+        options.nthreads = 0
+
+    if options.nthreads == 0:
+        try:
+            options.nthreads = int(os.environ['SAGE_NUM_THREADS_PARALLEL'])
+        except KeyError:
+            options.nthreads = 1
+
     from sage.doctest.control import DocTestController
-    DC = DocTestController(*parser.parse_args())
+    DC = DocTestController(options, args)
     err = DC.run()
     sys.exit(err)

and for the Sage library:

diff --git a/sage/doctest/control.py b/sage/doctest/control.py
--- a/sage/doctest/control.py
+++ b/sage/doctest/control.py
@@ -433,8 +433,9 @@ class DocTestController(SageObject):
             else:
                 nother += 1
         if nfiles + ndbsources + nother:
-            self.log("Doctesting %s."%(", ".join((["%s file%s"%(nfiles, "s" if nfiles > 1 else "")] if nfiles else []) +
-                                                 (["%s other sources"%nother] if nother else []))))
+            self.log("Doctesting %s"%(", ".join((["%s file%s"%(nfiles, "s" if nfiles > 1 else "")] if nfiles else []) +
+                                                 (["%s other sources"%nother] if nother else [])))
+                + " using %s threads."%self.options.nthreads if self.options.nthreads > 1 else ".")
             self.reporter = DocTestReporter(self)
             self.dispatcher = DocTestDispatcher(self)
             try:

Or something like that. I guess you should also print the number of threads when you print "Doctesting entire Sage library.", etc.