Update doctesting framework

[Robert Bradshaw]

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 #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/

and does most of the above (plus more which has been moved to followup tickets #12720 and #12722)

Apply attachment:12415_stderr_vs_51b5.patch, attachment:12415_doctest_fixes.patch​, attachment:12415_framework.patch​

Apply attachment:12415_script.patch​ to the scripts repo

Apply attachment:12415_sagenb_fixes.patch to the sagenb repo (fixes doctest errors)

Apply attachment:12415_spkg_bin_sage.patch​ to the root repo

[David Roe]

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

[Robert Bradshaw]

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

[R. Andrew Ohana]

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

[David Roe]

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

[David Roe]

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

[John Palmieri]

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

[David Roe]

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

[David Roe]

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.

[John Palmieri]

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

[John Palmieri]

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

[John Palmieri]

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

sage-runtests

@@ -5 +5 @@
 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 +49 @@
     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:

sage/doctest/control.py

@@ -433 +433 @@
             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.

[David Roe]

I've updated the option parsing to allow -p to take no options (though I used a callback so that the -p doesn't have to be last). I changed 12415_spkg_bin_sage.patch to be an actual patch against the root repository. I also made a small change to the debug functionality (printing the doctest that failed before dropping into the debugger).

Since I'm modifying four repositories, I'll just update the patches rather than creating lots of referee patches (unless the referee objects!).