Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wjp mannequin opened this issue Jan 19, 2010 · 17 comments
Closed

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

wjp mannequin opened this issue Jan 19, 2010 · 17 comments

Comments

@wjp
Copy link
Mannequin

wjp mannequin commented Jan 19, 2010

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.

Component: doctest coverage

Author: Willem Jan Palenstijn

Reviewer: Alex Ghitza

Merged: sage-4.3.2.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/7995

@wjp wjp mannequin added this to the sage-4.3.2 milestone Jan 19, 2010
@wjp wjp mannequin added t: tests labels Jan 19, 2010
@wjp
Copy link
Mannequin Author

wjp mannequin commented Jan 20, 2010

comment:2

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.

@wjp
Copy link
Mannequin Author

wjp mannequin commented Jan 20, 2010

@nthiery
Copy link
Contributor

nthiery commented Jan 20, 2010

comment:3

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

@nthiery
Copy link
Contributor

nthiery commented Jan 20, 2010

comment:4

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 

@nthiery
Copy link
Contributor

nthiery commented Jan 20, 2010

comment:5

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.

@wjp
Copy link
Mannequin Author

wjp mannequin commented Jan 20, 2010

comment:6

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?

@wjp
Copy link
Mannequin Author

wjp mannequin commented Jan 20, 2010

comment:7

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

@aghitza
Copy link

aghitza commented Jan 23, 2010

Author: Willem Jan Palenstijn

@aghitza
Copy link

aghitza commented Jan 23, 2010

comment:9

I played with this a bit and it works well.

@aghitza
Copy link

aghitza commented Jan 23, 2010

Reviewer: Alex Ghitza

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jan 23, 2010

comment:10

Merged in the script repository.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jan 23, 2010

Merged: sage-4.3.2.alpha0

@sagetrac-mvngu sagetrac-mvngu mannequin closed this as completed Jan 23, 2010
@wjp
Copy link
Mannequin Author

wjp mannequin commented Apr 13, 2010

comment:12

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.

@jhpalmieri
Copy link
Member

comment:13

See #8641 for a followup.

@jhpalmieri
Copy link
Member

comment:15

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants
@aghitza @nthiery @jhpalmieri and others