Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#7995 closed defect (fixed)

sage-test doesn't handle all of sage-doctest's return values

Reported by: wjp Owned by: tbd
Priority: major Milestone: sage-4.3.2
Component: doctest coverage Keywords:
Cc: Merged in: sage-4.3.2.alpha0
Authors: Willem Jan Palenstijn Reviewers: Alex Ghitza
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

The sage-doctest script returns some status info in its exit code, like if it was aborted with a KeyboardInterrupt. The sage-ptest script interprets this information, but sage-test mostly ignores it.

One symptom is that Ctrl-C-ing a sage -t run of multiple files doesn't work.

Attachments (1)

scripts_7995_sage-test_error_handling.patch (2.4 KB) - added by wjp 10 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 10 years ago by wjp

  • Status changed from new to needs_review

comment:2 Changed 10 years ago by wjp

Patch attached.

In the future we may want to merge the sage-ptest and sage-test scripts entirely. They have a lot of duplicate code and minor unnecessary differences.

comment:3 follow-up: Changed 10 years ago by nthiery

  • Status changed from needs_review to needs_work

I don't know much the doctest framework. But the code looks good, and this removes two seriously annoying misfeatures of doctests. Thanks so much for scratching this itch for me!

Quick remarks:

  • When there is an exception in the doctesting framework; will we get a traceback? That would be most handy!
  • in "... Check the process exit codes ...": codes -> code?
  • The following two comments seem contradictory. Or am I misunderstanding something?

# Return value in process exit code: # 0: all tests passed # 1: file not found # 2: KeyboardInterrupt? # 3: doctest process was terminated by a signal # 4: the doctesting framework raised an exception # 100: failed doctests ####################################################################

def test_code(filename):

# Return value in the doctest process exit code: # 0: everything passed # 1-253: number of failed doctests # 254: >= 254 doctests failed # 255: exception raised by doctesting framework

comment:4 in reply to: ↑ 3 ; follow-up: Changed 10 years ago by nthiery

Sorry, I screwed up my citation. Here it is again.

  • The following two comments seem contradictory. Or am I misunderstanding something?
 # Return value in process exit code: 
 # 0: all tests passed 
 # 1: file not found 
 # 2: KeyboardInterrupt 
 # 3: doctest process was terminated by a signal 
 # 4: the doctesting framework raised an exception 
 # 100: failed doctests 
 #################################################################### 

 def test_code(filename): 
     # Return value in the doctest process exit code: 
     # 0: everything passed 
     # 1-253: number of failed doctests 
     # 254: >= 254 doctests failed 
     # 255: exception raised by doctesting framework 

comment:5 in reply to: ↑ 4 Changed 10 years ago by nthiery

Replying to nthiery:

Sorry, I screwed up my citation. Here it is again.

Oops again. This remark is actually about #7993. I was looking at both, and got confused.

comment:6 Changed 10 years ago by wjp

Thanks for the feedback.

The sage-doctest script generates a new python file that runs the actual doctests, and runs this script in a seperate thread. The second block of process exit codes are for this 'inner' doctest process. I'll clarify the confusing comment (and fix the typo you mentioned).

As for the exception: you can get a traceback if you re-run with the -verbose. Do you think we should show them by default?

comment:7 Changed 10 years ago by wjp

I attached a new patch to #7993 that changes the exit code comments.

comment:8 Changed 9 years ago by AlexGhitza

  • Authors set to Willem Jan Palenstijn
  • Status changed from needs_work to needs_review

comment:9 Changed 9 years ago by AlexGhitza

  • Reviewers set to Alex Ghitza
  • Status changed from needs_review to positive_review

I played with this a bit and it works well.

comment:10 Changed 9 years ago by mvngu

  • Merged in set to sage-4.3.2.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

Merged in the script repository.

comment:11 Changed 9 years ago by ddrake

The patch here doesn't work; it exits sage-doctest with sys.exit(2), and then munges the error code with "err = err 256" -- that is, it does floor division by 256, so the 2 becomes 0 and KeyboardInterrupts? are never detected!

I hope to fix this while working on #8641.

comment:12 follow-up: Changed 9 years ago by wjp

I tested this, and it worked for me, as well as matching what the python docs say it does on unix systems. Does it not work on your system?

The python docs for os.system:

On Unix, the return value is the exit status of the process encoded in
the format specified for wait().

And for os.wait:

Wait for completion of a child process, and return a tuple containing
its pid and exit status indication: a 16-bit number, whose low byte is
the signal number that killed the process, and whose high byte is the
exit status (if the signal number is zero); the high bit of the low byte
is set if a core file was produced. Availability: Unix.

comment:13 Changed 9 years ago by jhpalmieri

See #8641 for a followup.

comment:14 in reply to: ↑ 12 Changed 9 years ago by ddrake

Replying to wjp:

I tested this, and it worked for me, as well as matching what the python docs say it does on unix systems. Does it not work on your system?

No, it doesn't. I'll work on this more, but I know that, on my Ubuntu 9.10 system, if I ctrl-c while running tests on a bunch of files, it doesn't interrupt the whole process. In sage-test, I have this:

print 'finished with %s, err: %s' % (F, err)
err = err // 256
print 'finished with %s, err: %s' % (F, err)

and here's what I get:

drake@sagenb:~/s/sage-4.3.5$ ./sage -t devel/sage/sage/combinat/t*.py
sage -t  "devel/sage/sage/combinat/tableau.py"              
^CKeyboardInterrupt -- interrupted after 2.3 seconds!
finished with devel/sage/sage/combinat/tableau.py, err: 2
finished with devel/sage/sage/combinat/tableau.py, err: 0
         [2.4 s]
sage -t  "devel/sage/sage/combinat/tools.py"                
finished with devel/sage/sage/combinat/tools.py, err: 0
finished with devel/sage/sage/combinat/tools.py, err: 0
         [1.6 s]
sage -t  "devel/sage/sage/combinat/tuple.py"                
finished with devel/sage/sage/combinat/tuple.py, err: 0
finished with devel/sage/sage/combinat/tuple.py, err: 0
         [2.2 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 6.2 seconds

I'm not sure why this isn't working.

comment:15 Changed 9 years ago by jhpalmieri

I put in a few print statements, like Dan did:

    print err,
    err = err // 256
    print err

On my iMac running OS X 10.6, when I hit ctrl-C, I see

515 2

which looks okay.

On sage.math, I see

2 0

which doesn't.

Note: See TracTickets for help on using tickets.