Opened 10 years ago
Closed 10 years ago
#7650 closed defect (fixed)
Fix sagenb doctesting
Reported by: | mpatel | Owned by: | was |
---|---|---|---|
Priority: | blocker | Milestone: | sage-4.3.1 |
Component: | notebook | Keywords: | |
Cc: | was, timdumol, mhansen, jason | Merged in: | sage-4.3.1.rc1 |
Authors: | Mitesh Patel | Reviewers: | Tim Dumol |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
We have not been doctesting SageNB files as often as we test Sage files.
Please apply
- trac_7650-scripts_doctest_force_lib_v5.patch to the scripts repository. This adds two options to
sage -t
:-sagenb
tests all SageNB files and-force_lib
tests files as if they live inSAGE_ROOT/devel/sage
, i.e., in the Sage library.
- trac_7650-sagenb_doctesting_v6.patch and trac_7650-reviewer_v2.patch to the sagenb repository. This fixes broken SageNB tests.
and replace SAGE_ROOT/makefile
with
Dependencies: Sage 4.3.1.alpha1, #7269.
Attachments (14)
Change History (37)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
sage -t -verbose "notebook.py" Traceback (most recent call last): File "/home/.sage//tmp/.doctest_notebook.py", line 18, in <module> from notebook import * File "/home/.sage/tmp/notebook.py", line 38, in <module> import css # style ImportError: No module named css [2.5 s] exit code: 1024 ---------------------------------------------------------------------- The following tests failed: sage -t -verbose "notebook.py" Total time for all tests: 2.5 seconds
Adding
import sys sys.path.append('/home/sage/notebook/sagenb/sagenb') sys.path.append('/home/sage/notebook/sagenb/sagenb/notebook')
to the top of notebook.py
allows testing to proceed. I'm not sure about a real solution.
comment:3 Changed 10 years ago by
- Priority changed from major to blocker
- Status changed from new to needs_review
Please note: The attachments (or their descriptions) are switched. I apologize for this.
Anyway, with the scripts repository patch, we can do, e.g.,
$ sage -t --force_lib sagenb/
With the sagenb repository patch applied to #7625 + #7483 + #7482 + #7514 + #7648, all tests but 3 pass. The failures are in sagenb/misc/sageinspect.py
(cf. #7514). The Selenium tests still pass.
Remarks:
- I hope I did not create false-negatives.
- This sage-notebook thread might be relevant to some tests that evaluate cells.
- I "untabified"
cell.py
,worksheet.py
,notebook.py
, andtwist.py
. - I added
'nodoctest'
tosimple/twist.py
, since at least one test appears to hang indefinitely. I don't know why. - Feel free to lower the priority.
comment:4 Changed 10 years ago by
- Cc mhansen added
comment:5 Changed 10 years ago by
Should we add, e.g.,
sage -t --force_lib $SAGE_ROOT/local/lib/python/site-packages/sagenb
to $SAGE_ROOT/makefile
? Or sage-test
?
comment:6 Changed 10 years ago by
- Status changed from needs_review to needs_work
I like this patch a lot. However, a nitpick: get rid of the / before tmp here:
SAGE_TESTDIR = os.path.join(DOT_SAGE, "/tmp")
Likewise, elsewhere in the patch there are a bunch of os.path.joins combined with explicit* slashes.
Regarding adding sage -t sagenb, etc., to sage-test or whatever: "yes!"
comment:7 Changed 10 years ago by
- Milestone changed from sage-4.3.1 to sage-4.3
comment:8 Changed 10 years ago by
- Status changed from needs_work to needs_review
With trac_7650-scripts_doctest_force_lib_v4.patch and an updated top-level makefile, we can do
make test
make ptest
sage -t -sagenb
sage -tp -sagenb
etc. Maybe we should unify the test scripts and use optparse?
comment:9 Changed 10 years ago by
- Status changed from needs_review to needs_work
The new options to sage -t
(-force_lib and -sagenb) are not documented in the help message. Please do so.
comment:10 Changed 10 years ago by
- Milestone changed from sage-4.3 to sage-4.3.1
I'm declaring a total feature freeze on sage-4.3.
comment:11 Changed 10 years ago by
Also: Be more agressive with os.path.join
-ery.
comment:12 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:13 Changed 10 years ago by
- Description modified (diff)
comment:14 Changed 10 years ago by
- Description modified (diff)
comment:15 Changed 10 years ago by
I've attached a rebased sagenb patch. I added """nodoctests"""
to the top of sagenb.misc.ipaddr
. It's also easy to make its doctest pass, but I haven't done this. Please let me know, if I should.
comment:16 Changed 10 years ago by
- Description modified (diff)
comment:17 follow-up: ↓ 18 Changed 10 years ago by
V5 makes it so a worksheet reconstruct
ed from the basic
representation of another has the same tags
and user_view
as the original.
Changed 10 years ago by
Fix interact doctests for 4.3.1.alpha2 (colors.py). Replaces previous sagenb patch.
comment:18 in reply to: ↑ 17 Changed 10 years ago by
Replying to mpatel:
V5 makes it so a worksheet
reconstruct
ed from thebasic
representation of another has the sametags
anduser_view
as the original.
Shouldn't this be put in a new ticket?
comment:19 Changed 10 years ago by
Yes, but it's already here. :) In the course of fixing many failed doctests, I noticed it and a few other small problems:
- In
run_notebook.notebook_twisted
: Replacednb.directory()
(not defined) withnb._dir
. - In
worksheet
: Replacedself.__collaborators
(AttributeError
) withself.collaborators()
in a few places. - In
worksheet.set_filename
:- self.__dir = os.path.join(self.notebook().worksheet_directory(), filename) + self.__dir = os.path.join(self.notebook()._dir, filename)
- In
worksheet.tags
:This ensures the tests ind = dict(self.__user_view) except AttributeError: self.user_view(self.owner()) - d = self.__user_view + d = copy.copy(self.__user_view) for user, val in d.iteritems(): - d[user] = [val] + if not isinstance(val, list): + d[user] = [val] return d
Worksheet.reconstruct_from_basic
pass and that callingtags
does not alter the current user view, e.g., turning1
into[1]
.
Also:
- Removed argument
verbose
(obsolete) fromCell.set_introspect_html
.
comment:20 Changed 10 years ago by
- Cc jason added
This bags a positive review from me, except for the few changes I've made in the reviewer patch. Can someone sign them off?
Changed 10 years ago by
A few documentation fixes (precision errors and style), and a couple of changes to try-except blocks to make it easier to transition to Python 3
comment:21 Changed 10 years ago by
- Description modified (diff)
- Reviewers set to Tim Dumol
- Status changed from needs_review to positive_review
V2 of the reviewer patch:
- Adds a missing dot (
.
) ininteract.py
. - Moves
# output depends on timezone
to the input statement inworksheet.py
. - [Pre-existing]
most be defined
-->must be defined
.
comment:22 Changed 10 years ago by
- Merged in set to sage-4.3.1.rc1
I applied the scripts patch and replaced makefile in SAGE_ROOT. Please post a sagenb-0.5.1.spkg
.
comment:23 Changed 10 years ago by
- Resolution set to fixed
- Status changed from positive_review to closed
Merged http://boxen.math.washington.edu/home/timdumol/sagenb-0.6.spkg into spkg/standard.
Part of the problem is that notebook directories now need to end in
.sagenb
.I'm working on a patch for this and whatever other problems I can fix.