Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#15120 closed bug (duplicate)

Fix doctest doctests in git layout.

Reported by: robertwb Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: Doctests Keywords:
Cc: Merged in:
Authors: Reviewers: Robert Bradshaw, Volker Braun
Report Upstream: N/A Work issues:
Branch: u/vbraun/ticket/15120 (Commits, GitHub, GitLab) Commit: e11727c3e33d4f8407324d83e13d69a3e14730f1
Dependencies: Stopgaps:

Status badges

Description (last modified by robertwb)

sage -t --long src/sage/doctest/control.py
**********************************************************************
File "src/sage/doctest/control.py", line 145, in sage.doctest.control.skipdir
Failed example:
    skipdir(os.path.join(SAGE_ROOT, "devel", "sagenb", "sagenb", "data"))
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/doctest/control.py", line 431, in sage.doctest.control.DocTestController.add_files
Failed example:
    DC.add_files()
Exception raised:
    Traceback (most recent call last):
      File "/Users/robertwb/sage/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/Users/robertwb/sage/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.doctest.control.DocTestController.add_files[10]>", line 1, in <module>
        DC.add_files()
      File "/Users/robertwb/sage/sage-git/local/lib/python2.7/site-packages/sage/doctest/control.py", line 455, in add_files
        from sage.misc.hg import hg_sage
    ImportError: No module named hg
**********************************************************************
2 items had failures:
   1 of  16 in sage.doctest.control.DocTestController.add_files
   1 of   4 in sage.doctest.control.skipdir
    [162 tests, 2 failures, 2.48 s]

Change History (29)

comment:1 Changed 8 years ago by robertwb

  • Description modified (diff)

comment:2 Changed 8 years ago by robertwb

  • Branch set to u/robertwb/ticket/15120
  • Created changed from 08/29/13 06:19:17 to 08/29/13 06:19:17
  • Modified changed from 08/29/13 06:25:57 to 08/29/13 06:25:57

comment:3 Changed 8 years ago by robertwb

  • Status changed from new to needs_review

comment:4 Changed 8 years ago by SimonKing

Trying to accomplish my first git review...

I see that you replace calls to hg_sage by rather "explicit" calls to subprocess, such as

  • src/sage/doctest/control.py

    diff --git a/src/sage/doctest/control.py b/src/sage/doctest/control.py
    index 9c0ed75..fa35315 100644
    a b class DocTestController(SageObject): 
    453453            self.files.extend(glob(opj(SAGE_SRC, 'doc', '[a-z][a-z]')))
    454454            self.options.sagenb = True
    455455        elif self.options.new:
    456             # Get all files changed in the working repo, as well as all
    457             # files in the top Mercurial queue patch.
    458             from sage.misc.hg import hg_sage
    459             out, err = hg_sage('status --rev qtip^', interactive=False, debug=False)
    460             if not err:
    461                 qtop = hg_sage('qtop', interactive=False, debug=False)[0].strip()
    462                 self.log("Doctesting files in mq patch " + repr(qtop))
    463             else:  # Probably mq isn't used
    464                 out, err = hg_sage('status', interactive=False, debug=False)
    465                 if not err:
    466                     self.log("Doctesting files changed since last hg commit")
    467                 else:
    468                     raise RuntimeError("failed to run hg status:\n" + err)
    469 
    470             for X in out.split('\n'):
    471                 tup = X.split()
    472                 if len(tup) != 2: continue
    473                 c, filename = tup
    474                 if c in ['M','A']:
    475                     filename = opj(SAGE_SRC, filename)
    476                     if not skipfile(filename):
    477                         self.files.append(filename)
     456            # Get all files changed in the working repo.
     457            import subprocess
     458            change = subprocess.check_output(["git",
     459                                              "--git-dir=" + SAGE_ROOT + "/.git",
     460                                              "--work-tree=" + SAGE_ROOT,
     461                                              "status",
     462                                              "--porcelain"])
     463            self.log("Doctesting files changed since last git commit")
     464            for line in change.split("\n"):
     465                if not line:
     466                    continue
     467                data = line.strip().split(' ')
     468                status, filename = data[0], data[-1]
     469                if (set(status).issubset("MARCU")
     470                    and filename.startswith("src/sage")
     471                    and (filename.endswith(".py") or filename.endswith(".pyx"))):
     472                    self.files.append(filename)
    478473        if self.options.sagenb:
    479474            if not self.options.all:
    480475                self.log("Doctesting the Sage notebook.")

Out of interest: Is "git_sage" already in the making? I.e., will it soon be possible to get the status by doing git_sage("status")?

comment:5 Changed 8 years ago by SimonKing

And question to you (Robert) as the author of the patchbot (if I am not mistaken on the authorship): Why is the patchbot stating:

git_branch: 	None

and thus is not running tests on your branch?

comment:6 Changed 8 years ago by SimonKing

Talking about doc tests: They are not finished on my laptop, yet. But I already got one error:

sage -t src/sage/doctest/forker.py
**********************************************************************
File "src/sage/doctest/forker.py", line 82, in sage.doctest.forker.init_sage
Failed example:
    print PrettyPrinter(settings={'wrap_line':True}).doprint(s)
Expected:
     29    28    27    26    25    24    23    22    21    20    19    18    17
    x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   +
    <BLANKLINE>
     16    15    14    13    12    11    10    9    8    7    6    5    4    3
    x   + x   + x   + x   + x   + x   + x   + x  + x  + x  + x  + x  + x  + x  + x
    <BLANKLINE>
    2
      + x
Got:
     29    28    27    26    25    24    23    22    21    20    19    18    17    16    15    14    13    12    11    10    9    8    7    6    5    4    3    2    
    x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x   + x  + x  + x  + x  + x  + x  + x  + x  + x
**********************************************************************
1 item had failures:
   1 of  14 in sage.doctest.forker.init_sage
    [436 tests, 1 failure, 18.51 s]

So, the problem is just wrong line break.

And now the real problem starts, namely the reviewing problem: If I understood correctly, you did two commits (Fix --new option for doctests. and Fix doctest doctest due to directory restructure.) here. Has this doctest error been introduced by your two commits, or by a "dependency"---and what is a "dependency", anyway???

comment:7 Changed 8 years ago by SimonKing

Fortunately there is "git blame". The doctest that is now failing was last edited by Volker, see #14266. But I suppose it used to not fail. Hence, I suppose that the reason for the failure is on a different ticket...

comment:8 Changed 8 years ago by SimonKing

make ptest gives me the following:

sage -t src/sage/doctest/forker.py  # 1 doctest failed
sage -t src/sage/misc/ascii_art.py  # 1 doctest failed
sage -t src/sage/dev/git_interface.py  # 10 doctests failed
sage -t src/sage/dev/sagedev.py  # 6 doctests failed

Has this ticket been supposed to fix all doctests?

comment:9 Changed 8 years ago by robertwb

It fixes the tests caused by "sage --testall" Hopefully we haven't introduced more breakage since then...but these at least should go on even if there's more work to do.

The sage dev scripts will provide some of this, but (by design) refuse to run in doctest mode. When I started working on this I was in a branch that didn't even have dev scripts, but the subprocess option wasn't that bad so I left it.

comment:10 follow-up: Changed 8 years ago by SimonKing

For the record: Here is the "background noise", i.e., the errors with "vanilla" public/sage-git/master, on my desktop computer.

sage -t --long src/sage/interfaces/maxima_abstract.py  # 2 doctests failed
sage -t --long src/sage/geometry/lattice_polytope.py  # 1 doctest failed
sage -t --long src/sage/doctest/control.py  # 1 doctest failed
sage -t --long src/sage/interfaces/interface.py  # 1 doctest failed
sage -t --long src/sage/dev/config.py  # 52 doctests failed
sage -t --long src/sage/dev/cmd_line_interface.py  # 22 doctests failed
sage -t --long src/sage/dev/git_interface.py  # 210 doctests failed
sage -t --long src/sage/dev/sagedev_wrapper.py  # 11 doctests failed
sage -t --long src/sage/dev/sagedev.py  # 571 doctests failed
sage -t --long src/sage/dev/test/config.py  # 2 doctests failed
sage -t --long src/sage/dev/test/sagedev.py  # 10 doctests failed
sage -t --long src/sage/dev/test/trac_interface.py  # 15 doctests failed
sage -t --long src/sage/dev/test/trac_server.py  # 2 doctests failed
sage -t --long src/sage/dev/test/server_proxy.py  # 61 doctests failed
sage -t --long src/sage/dev/test/user_interface.py  # 13 doctests failed
sage -t --long src/sage/dev/user_interface.py  # 25 doctests failed
sage -t --long src/sage/dev/trac_interface.py  # 113 doctests failed

For the record: I am a shocked that the soon-to-be fundament of sage development is broken to such extent. I am now trying to see whether the findings on my laptop can be confirmed on the desktop, namely that much less tests are failing with your patch/branch.

Question: Is the aim of this ticket to fix all tests?

comment:11 follow-up: Changed 8 years ago by SimonKing

I am working on my desktop because my laptop is broken. Question, Robert: While trying to use the dev scripts, I found a bug. Namely: I forgot to set up my git (e.g., I didn't store e-mail and real name). This should result in an error being raised. However, I get this traceback:

[git] git config user.name
Traceback (most recent call last):
  File "/mnt/local/king/SAGE/GIT/sage/src/bin/sage-dev", line 330, in <module>
    parser.run()
  File "/mnt/local/king/SAGE/GIT/sage/src/bin/sage-dev", line 322, in run
    method(*args,**kwds)
  File "/mnt/local/king/SAGE/GIT/sage/local/lib/python2.7/site-packages/sage/dev/sagedev_wrapper.py", line 140, in wrapped
    return f(*args, **kwargs)
  File "/mnt/local/king/SAGE/GIT/sage/local/lib/python2.7/site-packages/sage/dev/sagedev.py", line 518, in switch_ticket
    branch = self._new_local_branch_for_ticket(ticket)
  File "/mnt/local/king/SAGE/GIT/sage/local/lib/python2.7/site-packages/sage/dev...
...
  File "/mnt/local/king/SAGE/GIT/sage/local/lib/python2.7/site-packages/sage/dev/git_interface.py", line 358, in _check_user_email
    self._UI.normal("No real name has been set for git. This name shows up as the author for any commits you contribute to sage.")
AttributeError: 'CmdLineInterface' object has no attribute 'normal'

Is this bug already tracked/known to the sage-git people? I think I am currently not really able to create a ticket properly, with a slow internet connection and no usable laptop.

comment:12 in reply to: ↑ 11 Changed 8 years ago by robertwb

Replying to SimonKing:

I am working on my desktop because my laptop is broken. Question, Robert: While trying to use the dev scripts, I found a bug. Namely: I forgot to set up my git (e.g., I didn't store e-mail and real name). This should result in an error being raised. However, I get this traceback: ... Is this bug already tracked/known to the sage-git people? I think I am currently not really able to create a ticket properly, with a slow internet connection and no usable laptop.

I don't know; wouldn't hurt pinging the sage-git list.

comment:13 in reply to: ↑ 10 ; follow-up: Changed 8 years ago by robertwb

Replying to SimonKing:

For the record: Here is the "background noise", i.e., the errors with "vanilla" public/sage-git/master, on my desktop computer. ... For the record: I am a shocked that the soon-to-be fundament of sage development is broken to such extent.

I just ran the tests myself, got 79 files failing. Count me shocked as well.

I am now trying to see whether the findings on my laptop can be confirmed on the desktop, namely that much less tests are failing with your patch/branch.

Question: Is the aim of this ticket to fix all tests?

At this point, no, it's just to fix those two (which were the only ones on the github build_system branch). Looks like there's lots of further work to do.

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

Replying to robertwb:

Replying to SimonKing:

For the record: Here is the "background noise", i.e., the errors with "vanilla" public/sage-git/master, on my desktop computer. ... For the record: I am a shocked that the soon-to-be fundament of sage development is broken to such extent.

I just ran the tests myself, got 79 files failing. Count me shocked as well.

Really?? So, why is the number of failing files not the same as what I got? This makes things even worse, I'd say!

comment:15 Changed 8 years ago by robertwb

  • Authors set to Robert Bradshaw

comment:16 follow-up: Changed 8 years ago by git

  • Commit set to 6912669a7da6c5754663fb9a0ef6d29cdb73f38b

Branch pushed to git repo; I updated commit sha1. This was a forced push. Recent commits:

[changeset:6912669]Fix --new option for doctests.
[changeset:98d0840]Fix doctest doctest due to directory restructure.

comment:17 in reply to: ↑ 16 Changed 8 years ago by SimonKing

Replying to git:

Branch pushed to git repo; I updated commit sha1. This was a forced push. Recent commits:

[changeset:6912669]Fix --new option for doctests.
[changeset:98d0840]Fix doctest doctest due to directory restructure.

First problem: How do I update? sage -dev switch_ticket 15120? Or git pull trac if I am already in your branch?

Second problem: I get these errors in your branch.

sage -t --long src/sage/dev/sagedev.py  # 6 doctests failed
sage -t --long src/sage/interfaces/maxima_abstract.py  # 1 doctest failed
sage -t --long src/doc/en/constructions/plotting.rst  # 2 doctests failed
sage -t --long src/sage/dev/git_interface.py  # 10 doctests failed
sage -t --long src/sage/doctest/control.py  # 2 doctests failed
sage -t --long src/sage/dev/trac_interface.py  # 1 doctest failed

Why is this a problem, git-wise? Well, I learnt on sage-git that pulling your branch means I get something that is based on an old version of the "master" branch, i.e., a version of "master" that is not as terribly broken as the version mentioned in comment:10 and comment:13 - but still there are so many errors.

comment:18 Changed 8 years ago by SimonKing

Ouch. I am sorry, disregard my previous commit. I just did git branch and found

king@mpc622:/mnt/local/king/SAGE/GIT/sage$ git branch
* master
  public/sage-git/master

So, as it seems, I am not in your branch.

Doing sage -dev switch 15120 now...

comment:19 Changed 8 years ago by vbraun

Your changeset:6912669 has some weird changes to .gitignore. I've reverted that. The rest looks good to me and fixes the remaining doctest failures.

comment:20 Changed 8 years ago by vbraun

  • Branch changed from u/robertwb/ticket/15120 to u/vbraun/ticket/15120
  • Reviewers set to Volker Braun

comment:21 Changed 8 years ago by git

  • Commit changed from 6912669a7da6c5754663fb9a0ef6d29cdb73f38b to e11727c3e33d4f8407324d83e13d69a3e14730f1

Branch pushed to git repo; I updated commit sha1. New commits:

[changeset:e11727c]revert the change to .gitignore
[changeset:6912669]Fix --new option for doctests.
[changeset:98d0840]Fix doctest doctest due to directory restructure.

comment:22 Changed 8 years ago by SimonKing

Sorry for my lack of knowledge on git. What I did: ./sage -dev switch-ticket 15120, followed by "make". Afterwards, ./sage -dev did not work (so, I assume that the development scripts are in the master branch, but this ticket relies on a version of the master branch that predates the development scripts; correct?). Since I still found myself on commit 6912669, I did git pull, followed by git checkout e11727c. git log told me that now everything seems ok, I see Volker's recent addition on top of Robert's changes. So, I am now running make ptest.

But what I don't understand is this:

$ git branch -v
* (no branch)            e11727c revert the change to .gitignore
  master                 1456c52 Merge branch 'ticket/14482' into public/sage-git/master
  public/sage-git/master cf14c84 [behind 578] Merge branch 'ticket/14482' into public/sage-git/master
  ticket/15120           6912669 Fix --new option for doctests.

How can I make the branch ticket/15120 point to commit e11727c?

comment:23 Changed 8 years ago by vbraun

First of all, its perfectly fine to be in detached head mode. Especially if you don't want to make further commits on top of the given commit. You are just not in a branch, so what?

But, to answer your question: just delete the old branch and make a new one with your chosen head:

git branch -D ticket/15120
git checkout -b ticket/15120 HEAD

Alternatively, don't enter detached head mode to start with:

git checkout ticket/15120
git pull trac u/vbraun/ticket/15120

comment:24 Changed 8 years ago by SimonKing

Volker, thank you for your hints on making my git repository nicer.

Here are the errors that I still get:

sage -t src/sage/tests/cmdline.py  # 3 doctests failed
sage -t src/sage/interfaces/r.py  # 183 doctests failed
sage -t src/sage/interfaces/expect.py  # 7 doctests failed
sage -t src/sage/misc/sageinspect.py  # 1 doctest failed
sage -t src/sage/misc/interpreter.py  # 4 doctests failed
sage -t src/doc/en/prep/Quickstarts/Statistics-and-Distributions.rst  # 6 doctests failed
sage -t src/sage/interfaces/interface.py  # 4 doctests failed
sage -t local/lib/python2.7/site-packages/sagenb-0.10.7.2-py2.7.egg/sagenb/misc/support.py  # 1 doctest failed
sage -t src/sage/stats/r.py  # 1 doctest failed

In comment:19, you state that it "fixes the remaining doctest failures"---after merging the current state of the master, or how? I did not merge the current master.

comment:25 Changed 8 years ago by vbraun

That's what I did, I merged the current master (public/sage-git/master ), then ran the doctests, and then threw away the merge commit. All tests passed after the merge. Since there is no merge conflict there is no point in me pushing the merge, the release manager will do that when the ticket is merged.

comment:26 Changed 8 years ago by vbraun

  • Status changed from needs_review to positive_review

I guess nobody dares to review the obvious revert of the gitignore change, fine. I'll do it myself then :P

comment:27 Changed 8 years ago by ohanar

  • Milestone changed from sage-5.12 to sage-duplicate/invalid/wontfix

I've merged this into the build_system branch at #14480, so I'm marking this as a duplicate so that we can close this ticket now.

Robert: the build_system branch should now have all doctests pass, so you shouldn't need to maintain a separate branch for the patchbot.

comment:28 Changed 8 years ago by jdemeyer

  • Resolution set to duplicate
  • Status changed from positive_review to closed

comment:29 Changed 8 years ago by jdemeyer

  • Authors Robert Bradshaw deleted
  • Reviewers changed from Volker Braun to Robert Bradshaw, Volker Braun
Note: See TracTickets for help on using tickets.