Opened 10 years ago
Last modified 9 years ago
#12415 closed enhancement
Update doctesting framework — at Version 28
Reported by: | robertwb | Owned by: | mvngu |
---|---|---|---|
Priority: | major | Milestone: | sage-5.9 |
Component: | doctest framework | Keywords: | |
Cc: | kini, ohanar, jhpalmieri | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #13147,#13146, #13145, #12723, #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384 | Stopgaps: |
Description (last modified by )
There are several improvements that would be good to make, including but not limited to:
- Forking rather than starting up a new Sage each time.
- Customizable comparison rather than mutating doctests into new doctests (e.g. followup to #10952).
- Eliminate need to write temporary files for every doctested file.
- Constant headaches caused by the difference in doctesting library vs. non-library files.
- 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
Change History (28)
comment:1 Changed 10 years ago by
- Description modified (diff)
comment:2 Changed 10 years ago by
- Description modified (diff)
comment:3 Changed 10 years ago by
- Description modified (diff)
comment:4 Changed 10 years ago by
- Description modified (diff)
comment:5 Changed 10 years ago by
- Description modified (diff)
comment:6 Changed 10 years ago by
- Dependencies set to #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384
comment:7 Changed 10 years ago by
- Dependencies changed from #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384 to #12723, #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384
comment:8 Changed 10 years ago by
- Cc kini added
comment:9 Changed 10 years ago by
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?)
comment:10 Changed 10 years ago by
- Cc ohanar added
comment:11 Changed 10 years ago by
apparently even I forget my trac username is not what it should be
comment:12 Changed 10 years ago by
- Dependencies changed from #12723, #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384 to #13145, #12723, #12392, #12393, #12395, #12396, #12397, #12381, #12382, #12383, #12384
comment:13 Changed 10 years ago by
- Dependencies changed 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
comment:14 Changed 10 years ago by
- Dependencies changed 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
comment:15 Changed 10 years ago by
I've moved the patches dealing with temporary files to #13147.
comment:16 Changed 10 years ago by
- Cc jhpalmieri added
comment:17 Changed 10 years ago by
- Description modified (diff)
comment:18 Changed 10 years ago by
- Description modified (diff)
comment:19 Changed 10 years ago by
- Status changed from new to needs_review
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.
comment:20 Changed 10 years ago by
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
comment:21 Changed 10 years ago by
comment:22 Changed 10 years ago by
- Description modified (diff)
comment:23 Changed 10 years ago by
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.
comment:24 Changed 10 years ago by
Regarding 12415_spkg_bin_sage.patch, the file in question should be part of the root repo.
comment:25 Changed 10 years ago by
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())
.
comment:26 Changed 10 years ago by
I would suggest the following two changes: for the scripts repo:
-
sage-runtests
diff --git a/sage-runtests b/sage-runtests
a b import optparse, os, sys 5 5 if __name__ == "__main__": 6 6 parser = optparse.OptionParser() 7 7 8 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())")8 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())") 9 9 parser.add_option("--serial", action="store_true", default=False, help="run tests in a single process in series") 10 10 parser.add_option("--timeout", type=int, default=-1, help="timeout (in seconds) for doctesting one file") 11 11 parser.add_option("-a", "--all", action="store_true", default=False, help="test all files in the Sage library") … … if __name__ == "__main__": 49 49 parser.add_option("--stats_path", "--stats-path", default=os.path.join(os.path.expanduser("~/.sage/timings2.json")), \ 50 50 help="path to a json dictionary for the latest run storing a timing for each file") 51 51 52 options, args = parser.parse_args() 53 try: 54 options.nthreads = int(options.nthreads) 55 except ValueError: 56 args.insert(0, options.nthreads.strip()) 57 options.nthreads = 0 58 59 if options.nthreads == 0: 60 try: 61 options.nthreads = int(os.environ['SAGE_NUM_THREADS_PARALLEL']) 62 except KeyError: 63 options.nthreads = 1 64 52 65 from sage.doctest.control import DocTestController 53 DC = DocTestController( *parser.parse_args())66 DC = DocTestController(options, args) 54 67 err = DC.run() 55 68 sys.exit(err)
and for the Sage library:
-
sage/doctest/control.py
diff --git a/sage/doctest/control.py b/sage/doctest/control.py
a b class DocTestController(SageObject): 433 433 else: 434 434 nother += 1 435 435 if nfiles + ndbsources + nother: 436 self.log("Doctesting %s."%(", ".join((["%s file%s"%(nfiles, "s" if nfiles > 1 else "")] if nfiles else []) + 437 (["%s other sources"%nother] if nother else [])))) 436 self.log("Doctesting %s"%(", ".join((["%s file%s"%(nfiles, "s" if nfiles > 1 else "")] if nfiles else []) + 437 (["%s other sources"%nother] if nother else []))) 438 + " using %s threads."%self.options.nthreads if self.options.nthreads > 1 else ".") 438 439 self.reporter = DocTestReporter(self) 439 440 self.dispatcher = DocTestDispatcher(self) 440 441 try:
Or something like that. I guess you should also print the number of threads when you print "Doctesting entire Sage library.", etc.
comment:27 Changed 10 years ago by
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!).
comment:28 Changed 10 years ago by
- Description modified (diff)
I'm working on this. E-mail me if you want to collaborate.