Opened 9 years ago

Closed 6 years ago

Last modified 4 years ago

#10589 closed defect (fixed)

sage -fixdoctests doesn't work correctly

Reported by: mderickx Owned by: mvngu
Priority: minor Milestone: sage-5.13
Component: scripts Keywords: days45
Cc: Merged in: sage-5.13.beta1
Authors: Andrew Mathas Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #12415 Stopgaps:

Description (last modified by chapoton)

This patch fixes the following problems with the command line fixdoctests option:

  • correctly deals with tests for which output it expected but none is generated, or for which no output is expected by some is generated.
  • makes fixdoctests play nicely with the changes introduced in #12415
  • adds some tests to sage/tests/cmdline.py
  • makes the script work slightly better with tests that produce output with varying indentation
  • cleans up the code of sage-fixdoctests
  • sage-fixdoctests now checks to see if it can write to the specified output directory and if not it prompts the user for a better place to put the file
  • the script now takes an optional argument --long so that fixdoctests can also massage --long doctests
  • there is an updated usage message for fixdoctests printed by sage --advanced
  • there is an updated usage message for fixdoctests in the reference manual

---

Original ticket:

To reproduce this remove all but the first test result from the patch at: #10568

maarten-derickxs-macbook-pro:sage maarten$ sage -t matrix/matrix_sparse.pyx
sage -t  "devel/sage-main/sage/matrix/matrix_sparse.pyx"    
**********************************************************************
File "/Applications/sage-4.6.rc0/devel/sage-main/sage/matrix/matrix_sparse.pyx", line 301:
    sage: (2/3)*M
Expected nothing
Got:
    [   0  2/3  4/3    2  8/3 10/3]
    [   4 14/3 16/3    6 20/3 22/3]
    [   8 26/3 28/3   10 32/3 34/3]
**********************************************************************
File "/Applications/sage-4.6.rc0/devel/sage-main/sage/matrix/matrix_sparse.pyx", line 302:
    sage: 7*M
Expected nothing
Got:
    [  0   7  14  21  28  35]
    [ 42  49  56  63  70  77]
    [ 84  91  98 105 112 119]
**********************************************************************
File "/Applications/sage-4.6.rc0/devel/sage-main/sage/matrix/matrix_sparse.pyx", line 303:
    sage: (1/4)*M
Expected nothing
Got:
    [   0  1/4  1/2  3/4    1  5/4]
    [ 3/2  7/4    2  9/4  5/2 11/4]
    [   3 13/4  7/2 15/4    4 17/4]
**********************************************************************
File "/Applications/sage-4.6.rc0/devel/sage-main/sage/matrix/matrix_sparse.pyx", line 306:
    sage: m==(97/42)*(42/97*m)
Expected nothing
Got:
    True
**********************************************************************
1 items had failures:
   4 of   9 in __main__.example_6
***Test Failed*** 4 failures.
For whitespace errors, see the file /Users/maarten/.sage//tmp/.doctest_matrix_sparse.py
 [12.5 s]
 
----------------------------------------------------------------------
The following tests failed:


sage -t  "devel/sage-main/sage/matrix/matrix_sparse.pyx"
Total time for all tests: 12.6 seconds
maarten-derickxs-macbook-pro:sage maarten$ sage -fixdoctests matrix/matrix_sparse.pyx

The bug was caused by the script not allowing for the cases when either the expected or computed returns from the doctests were empty. The attached patch does this properly and also cleans up the script a little (well, a lot, actually).

Note that the script lives in $SAGE_ROOT/local/bin which is not part of the standard repository. The (new) doctests for the script live in sage/tests/cmdline.py

Apply: trac_10589--doctests_for_fixdoctests-am.patch

Apply to scripts repository: trac_10589--fixdoctest_failures-am.patch (modifies $SAGE_ROOT/local/bin)

Applt to root repository: trac_10589--updating_fixdoctests_command_line_options-am.patch (modifies $SAGE_ROOT/spkg/bin)

Attachments (3)

trac_10589--updating_fixdoctests_command_line_options-am.patch (1.9 KB) - added by andrew.mathas 6 years ago.
Fixing help strings
trac_10589--fixdoctest_failures-am.patch (7.2 KB) - added by andrew.mathas 6 years ago.
Makes fixdoctests use difflib to generate diffs
trac_10589--doctests_for_fixdoctests-am.patch (4.6 KB) - added by andrew.mathas 6 years ago.
Makes fixdoctests use difflib to generate diffs

Download all attachments as: .zip

Change History (69)

comment:1 Changed 7 years ago by andrew.mathas

  • Authors set to Andrew Mathas
  • Description modified (diff)
  • Keywords sage45 added
  • Status changed from new to needs_review

comment:2 Changed 7 years ago by andrew.mathas

  • Description modified (diff)
  • Milestone set to sage-5.8

comment:3 Changed 7 years ago by andrew.mathas

  • Description modified (diff)

comment:4 Changed 7 years ago by andrew.mathas

  • Description modified (diff)

comment:5 Changed 7 years ago by andrew.mathas

  • Description modified (diff)

comment:6 follow-up: Changed 7 years ago by jhpalmieri

To doctest this, you could add some tests to devel/sage/sage/tests/cmdline.py

comment:7 Changed 7 years ago by andrew.mathas

  • Keywords days45 added; sage45 removed

comment:8 in reply to: ↑ 6 Changed 7 years ago by andrew.mathas

Replying to jhpalmieri:

To doctest this, you could add some tests to devel/sage/sage/tests/cmdline.py

Thanks John. I will have a look at this and add some tests.

Last edited 7 years ago by andrew.mathas (previous) (diff)

comment:9 Changed 7 years ago by andrew.mathas

  • Status changed from needs_review to needs_work

comment:10 Changed 7 years ago by roed

  • Component changed from doctest to doctest framework

comment:11 Changed 7 years ago by andrew.mathas

  • Dependencies set to 12415
  • Status changed from needs_work to needs_review

Updated fixdoctests patch so that it works with the new doctest framework in #12415 and added some tests to sage/tests/cmdline.py for the script

comment:12 Changed 7 years ago by andrew.mathas

  • Description modified (diff)

comment:13 Changed 7 years ago by andrew.mathas

  • Description modified (diff)

comment:14 Changed 7 years ago by andrew.mathas

  • Description modified (diff)

comment:15 Changed 7 years ago by andrew.mathas

  • Description modified (diff)

comment:16 Changed 7 years ago by andrew.mathas

  • Description modified (diff)

comment:17 follow-up: Changed 7 years ago by roed

Shouldn't that be the scripts repo?

comment:18 in reply to: ↑ 17 Changed 7 years ago by andrew.mathas

  • Description modified (diff)

Replying to roed:

Shouldn't that be the scripts repo?

Yes, thanks. Wasn't sure what the right name was, just knew where to find it...

comment:19 Changed 7 years ago by jdemeyer

  • Component changed from doctest framework to scripts
  • Dependencies changed from 12415 to #12415

comment:20 Changed 6 years ago by benjaminfjones

I used the patch here to solve problems I was getting with sage -fixdoctests. Thanks, hope this can get reviewed and merged soon!

comment:21 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:22 Changed 6 years ago by andrew.mathas

  • Description modified (diff)

comment:23 follow-up: Changed 6 years ago by tscrim

Looks good to me minus two minors thing:

  • you're missing the :: on line 405 in tests/cmdline.py,
  • could you de-indent the AUTHORS: block in sage-fixdoctests and align the extra text by 2 spaces.

Thanks Andrew.

Last edited 6 years ago by tscrim (previous) (diff)

comment:24 in reply to: ↑ 23 Changed 6 years ago by andrew.mathas

Replying to tscrim:

Looks good to me minus two minors thing:

  • you're missing the :: on line 405 in tests/cmdline.py,
  • could you de-indent the AUTHORS: block in sage-fixdoctests and align the extra text by 2 spaces.

Thanks Travis, I have fixed both of these.

Andrew

comment:25 Changed 6 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Then it's a positive review. Thanks Andrew.

comment:26 follow-up: Changed 6 years ago by jdemeyer

  • Status changed from positive_review to needs_work

The file trac_10589--fixdoctest_failures-am.patch contains garbage at the start, please fix that.

comment:27 in reply to: ↑ 26 Changed 6 years ago by andrew.mathas

Replying to jdemeyer:

The file trac_10589--fixdoctest_failures-am.patch contains garbage at the start, please fix that.

Sorry. Fixed.

comment:28 Changed 6 years ago by andrew.mathas

  • Status changed from needs_work to positive_review

comment:29 follow-up: Changed 6 years ago by jdemeyer

  • Status changed from positive_review to needs_work

I get

sage -t --long devel/sage/sage/tests/cmdline.py
**********************************************************************
File "devel/sage/sage/tests/cmdline.py", line 414, in sage.tests.cmdline.test_executable
Failed example:
    ret
Expected:
    0
Got:
    1
**********************************************************************
File "devel/sage/sage/tests/cmdline.py", line 418, in sage.tests.cmdline.test_executable
Failed example:
    print output[output.find('     SAGE: 1+1'):output.find(' \"\"\"')-1]
Expected:
         SAGE: 1+1              # incorrect output
    -    3
    +    2
         SAGE: m=matrix(ZZ,3)   # output when none is expected
    +    SAGE: (2/3)*m          # no output when it is expected
    +    [0 0 0]
         [0 0 0]
         [0 0 0]
    -    [1 0 0]
    -    SAGE: (2/3)*m          # no output when it is expected
         SAGE: mu=PartitionTuple([[4,4],[3,3,2,1],[1,1]])   # output when none is expected
    -    [4, 4, 3, 3, 2, 1, 1]
         SAGE: mu.pp()          # uneven indentation
    -    ****
    -    ****
    +       ****   ***   *
    +       ****   ***   *
    +              **
    +              *
         SAGE: PartitionTuples.global_options(convention="French")
         SAGE: mu.pp()         # fix doctest with uneven indentation
    +    *
    +    **
    +    ****   ***   *
    +    ****   ***   *
         SAGE: PartitionTuples.global_options.reset()
Got:
    <BLANKLINE>
**********************************************************************

comment:30 Changed 6 years ago by jdemeyer

  • Description modified (diff)

comment:31 in reply to: ↑ 29 Changed 6 years ago by andrew.mathas

  • Status changed from needs_work to positive_review

Replying to jdemeyer:

I get

sage -t --long devel/sage/sage/tests/cmdline.py
**********************************************************************
File "devel/sage/sage/tests/cmdline.py", line 414, in sage.tests.cmdline.test_executable
Failed example:
   
 ...

Hi Jeroen,

I am fairly sure that this failure is because the patch in the scripts directory has not been applied - at least, I get this error if the patch trac_10589--fixdoctest_failures-am.patch has not been applied to the scripts repository and I do not get it when it is applied.

Can you please confirm that both patches have been applied correctly. I'm putting the status back to positive review as I am not sure if you will read this otherwise.

Andrew

comment:32 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.12 to sage-5.13

comment:33 Changed 6 years ago by jdemeyer

I'm pretty sure I did apply both patches, but I'll try again...

comment:34 Changed 6 years ago by jdemeyer

  • Status changed from positive_review to needs_work

After adding a doctest line

sage: print err

(which I guess all cmdline.py tests should have):

**********************************************************************
File "devel/sage/sage/tests/cmdline.py", line 416, in sage.tests.cmdline.test_executable
Failed example:
    print err
Expected nothing
Got:
    sh: cannot create .tmp: Permission denied
    Traceback (most recent call last):
      File "/scratch/release/merger/sage-5.13.beta0/local/bin/sage-fixdoctests", line 35, in <module>
        doc_out = open(doc_out).read()
    IOError: [Errno 2] No such file or directory: '.tmp'
    <BLANKLINE>
**********************************************************************

So the problem is that sage -fixdoctests writes to the current working directory, which isn't guaranteed to be writable.

Also: please use --fixdoctests everywhere.

comment:35 Changed 6 years ago by andrew.mathas

  • Status changed from needs_work to positive_review

Thanks for identifying the problem Jeroen as I couldn't see this the way I was testing it. I've fixed both of these points. Andrew

comment:36 Changed 6 years ago by jdemeyer

  • Status changed from positive_review to needs_work

comment:37 follow-up: Changed 6 years ago by jdemeyer

  • Status changed from needs_work to needs_review

I think these changes should be reviewed. Travis, can you do that?

comment:38 in reply to: ↑ 37 Changed 6 years ago by andrew.mathas

Replying to jdemeyer:

I think these changes should be reviewed. Travis, can you do that?

Hi Travis,

FYI, the trivial changes made were:

  • in trac_10589--doctests_for_fixdoctests-am.patch​ two instances of -fixdoctests were changes to --fixdoctests and and print ret was changed to print err
  • in trac_10589--fixdoctest_failures-am.patch the output is sent to doc_out=tmp_dir()+'.tmp' rather than just .tmp, which required a new import statement (this was an error in the original sage-fixdoctests which wasn't picked up when it was first added to sage...).

Andrew

comment:39 follow-up: Changed 6 years ago by tscrim

Hey Andrew,

Is there some way we could make this behave better when we don't have write permissions to a given directory, such as output query the user or just output to the console?

Hey Jeroen,

Thanks for noticing that, I never would have thought to check write permissions of the current directory.

Best,
Travis

comment:40 in reply to: ↑ 39 ; follow-up: Changed 6 years ago by andrew.mathas

Replying to tscrim:

Hey Andrew,

Is there some way we could make this behave better when we don't have write permissions to a given directory, such as output query the user or just output to the console?

Hi Travis,

This already happens. The problem that Jeroen found concerns a tempory file which the sage-fixdoctests script uses to dump the current state of the doc-tests. This file is now written to sage's tmp_dir() which is always writable.

The only other file that sage-fixdoctests writes is the replacement source file. By default this is put in the same directory as the original source, however, the user can supply an optional argument to have the file written elsewhere:

Usage: sage -fixdoctests <source file> [doctest output]
Creates a file <source file>.out that passes the doctests (modulo any raised exceptions)

Having said this, I just chcked and this optional argument is not mentioned under sage -advanced, so probably I should add it there. I could also make the script more user friendy and have it remind the user of this when it is unable to write its output file. Is this what you have in mind?

Andrew

Last edited 6 years ago by andrew.mathas (previous) (diff)

comment:41 in reply to: ↑ 40 Changed 6 years ago by tscrim

Replying to andrew.mathas:

This already happens. The problem that Jeroen found concerns a tempory file which the sage-fixdoctests script uses to dump the current state of the doc-tests. This file is now written to sage's tmp_dir() which is always writable.

The only other file that sage-fixdoctests writes is the replacement source file. By default this is put in the same directory as the original source, however, the user can supply an optional argument to have the file written elsewhere:

Usage: sage -fixdoctests <source file> [doctest output]
Creates a file <source file>.out that passes the doctests (modulo any raised exceptions)

Having said this, I just chcked and this optional argument is not mentioned under sage -advanced, so probably I should add it there. I could also make the script more user friendy and have it remind the user of this when it is unable to write its output file. Is this what you have in mind?

Hey Andrew,

Yes, that would be good.

Thanks,
Travis

comment:42 follow-up: Changed 6 years ago by jdemeyer

I confirm that the new patches seem to work (but I haven't looked at the code).

comment:43 in reply to: ↑ 42 Changed 6 years ago by andrew.mathas

Replying to jdemeyer:

I confirm that the new patches seem to work (but I haven't looked at the code).

Thanks! I haven't yet incororated Travis' suggestions above. Btw, can any one tell me where the sage usage message lives so that I can mention the optional argument to fixdoctests there?

comment:44 Changed 6 years ago by jhpalmieri

It's in SAGE_ROOT/spkg/bin/sage. You should also update the documentation at devel/sage/doc/en/reference/cmd/options.rst.

comment:45 Changed 6 years ago by andrew.mathas

  • Description modified (diff)

OK, I have just uploaded a new version of the patch, which now consists of three files for three different repositories (main, scripts and spkg). The new features of the patch are:

  • sage-fixdoctests now checks to see if it can write to the specified output directory and if not it prompts the user for a better place to put the file
  • the script now takes an optional argument --long so that fixdoctests can also massage --long doctests
  • there is an updated usage message for fixdoctests printed by sage --advanced
  • there is an updated usage message for fixdoctests in the reference manual

Ready for review...

comment:46 follow-up: Changed 6 years ago by tscrim

One very minor point, could you put a space after the colon on line 122 in the fixdoctest file? I prefer my prompts like:

prompt: foo

as opposed to:

prompt:foo

After that, you can set this to positive review. Thanks.

comment:47 in reply to: ↑ 46 Changed 6 years ago by andrew.mathas

  • Status changed from needs_review to positive_review

Replying to tscrim:

One very minor point, could you put a space after the colon on line 122 in the fixdoctest file? I prefer my prompts like:

After that, you can set this to positive review. Thanks.

Thanks very much Travis. Both done!

Andrew

comment:48 Changed 6 years ago by jdemeyer

  • Description modified (diff)

comment:49 follow-up: Changed 6 years ago by jdemeyer

  • Status changed from positive_review to needs_work

There is an obvious doctest failure, which should have been caught both by the author and reviewer here:

sage -t devel/sage/sage/tests/cmdline.py
**********************************************************************
File "devel/sage/sage/tests/cmdline.py", line 178, in sage.tests.cmdline.test_executable
Failed example:
    err
Expected:
    ''
Got:
    '/scratch/release/merger/sage-5.13.beta0/spkg/bin/sage: line 155: file.py: command not found\n/scratch/release/merger/sage-5.13.beta0/spkg/bin/sage: line 155: outpu
t_file: command not found\n/scratch/release/merger/sage-5.13.beta0/spkg/bin/sage: line 156: file.py.out: command not found\n/scratch/release/merger/sage-5.13.beta0/spkg
/bin/sage: line 157: --long: command not found\n'
**********************************************************************

Changed 6 years ago by andrew.mathas

Fixing help strings

comment:50 in reply to: ↑ 49 ; follow-up: Changed 6 years ago by andrew.mathas

Replying to jdemeyer:

There is an obvious doctest failure, which should have been caught both by the author and reviewer here:

Yep, agreed. Sorry. Fixed.

Jeroen, I always get the feeling you don't like me.

A.

comment:51 in reply to: ↑ 50 ; follow-up: Changed 6 years ago by jdemeyer

Replying to andrew.mathas:

Jeroen, I always get the feeling you don't like me.

It is true that failures like these are annoying to me, because they make me lose time while being completely avoidable. On the other hand, it is nothing personal (usually, I don't even look at ticket's authors). I certainly didn't mean to offend, and I apologize if I did.

comment:52 in reply to: ↑ 51 Changed 6 years ago by andrew.mathas

Replying to jdemeyer:

Replying to andrew.mathas:

Jeroen, I always get the feeling you don't like me.

It is true that failures like these are annoying to me, because they make me lose time while being completely avoidable. On the other hand, it is nothing personal (usually, I don't even look at ticket's authors). I certainly didn't mean to offend, and I apologize if I did.

No, not offended as I completely understand: it annoys me when I do this as well.

Apologies for wasting your time annd Travis' (and mine), as I am sure we all have better thing to do.

A.

comment:53 follow-up: Changed 6 years ago by tscrim

  • Status changed from needs_work to positive_review

Sorry about that Jeroen. I'm somewhat surprised that changes to the output of the --advanced broke that particular doctest. With Andrew's new patch it is fixed:

travis@Kristine-Desktop:~/sage-5.12.beta5/devel/sage-combinat/sage/tests$ sage -t --long ../tests/cmdline.py 
Running doctests with ID 2013-10-04-07-25-10-694770dc.
Doctesting 1 file.
sage -t --long cmdline.py
    [213 tests, 95.98 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 96.4 seconds
    cpu time: 0.4 seconds
    cumulative wall time: 96.0 seconds

comment:54 in reply to: ↑ 53 ; follow-up: Changed 6 years ago by jdemeyer

Replying to tscrim:

Sorry about that Jeroen. I'm somewhat surprised that changes to the output of the --advanced broke that particular doctest.

There reason is that ` is a special character for the shell.

comment:55 in reply to: ↑ 54 Changed 6 years ago by tscrim

Replying to jdemeyer:

Replying to tscrim:

Sorry about that Jeroen. I'm somewhat surprised that changes to the output of the --advanced broke that particular doctest.

There reason is that ` is a special character for the shell.

Ah. Thanks.

comment:56 follow-up: Changed 6 years ago by jdemeyer

  • Status changed from positive_review to needs_work

The diff output isn't portable, on Solaris:

sage -t --long devel/sage/sage/tests/cmdline.py
**********************************************************************
File "devel/sage/sage/tests/cmdline.py", line 417, in sage.tests.cmdline.test_executable
Failed example:
    print output[output.find('     SAGE: 1+1'):output.find(' \"\"\"')-1]
Expected:
         SAGE: 1+1              # incorrect output
    -    3
    +    2
         SAGE: m=matrix(ZZ,3)   # output when none is expected
    +    SAGE: (2/3)*m          # no output when it is expected
    +    [0 0 0]
         [0 0 0]
         [0 0 0]
    -    [1 0 0]
    -    SAGE: (2/3)*m          # no output when it is expected
         SAGE: mu=PartitionTuple([[4,4],[3,3,2,1],[1,1]])   # output when none is expected
    -    [4, 4, 3, 3, 2, 1, 1]
         SAGE: mu.pp()          # uneven indentation
    -    ****
    -    ****
    +       ****   ***   *
    +       ****   ***   *
    +              **
    +              *
         SAGE: PartitionTuples.global_options(convention="French")
         SAGE: mu.pp()         # fix doctest with uneven indentation
    +    *
    +    **
    +    ****   ***   *
    +    ****   ***   *
         SAGE: PartitionTuples.global_options.reset()
Got:
         SAGE: 1+1              # incorrect output
    -    3
    +    2
         SAGE: m=matrix(ZZ,3)   # output when none is expected
    +    SAGE: (2/3)*m          # no output when it is expected
         [0 0 0]
         [0 0 0]
    -    [1 0 0]
    -    SAGE: (2/3)*m          # no output when it is expected
    +    [0 0 0]
         SAGE: mu=PartitionTuple([[4,4],[3,3,2,1],[1,1]])   # output when none is expected
    -    [4, 4, 3, 3, 2, 1, 1]
         SAGE: mu.pp()          # uneven indentation
    -    ****
    -    ****
    +       ****   ***   *
    +       ****   ***   *
    +              **
    +              *
         SAGE: PartitionTuples.global_options(convention="French")
         SAGE: mu.pp()         # fix doctest with uneven indentation
    +    *
    +    **
    +    ****   ***   *
    +    ****   ***   *
         SAGE: PartitionTuples.global_options.reset()
**********************************************************************

comment:57 in reply to: ↑ 56 ; follow-up: Changed 6 years ago by andrew.mathas

Replying to jdemeyer:

The diff output isn't portable, on Solaris:

This is going to be tricky as I don't have access to a solaris machine so I can't test this. The only difference between the two diffs is where the + [0 0 0] appears.

It seems that the problem may be that -fixdoctests uses a universal diff:

os.system('diff -u %s %s | more'%(test_file, test_output))

Aunty google suggests that a context diff may work better:

os.system('diff -c %s %s | more'%(test_file, test_output))

Is anyone able to confirm this?

Looking at this there is also an issue because diff may not be installed on the system, or it may not be in the users PATH. The script should try and trap the resulting errors in these cases...although I'll need to find appropriate exceptions to look for. Of course, in all of these cases this test will fail but I don't see a way around this. Having diff installed presumably isn't, and shouldn't be, a requirement for sage.

Is there a way to mark tests for the doc-test suite to ignore a test on certain systems?

A.

comment:58 in reply to: ↑ 57 Changed 6 years ago by andrew.mathas

Please ignore my queries: I have just discovered difflib which should fix both issues. A.

Changed 6 years ago by andrew.mathas

Makes fixdoctests use difflib to generate diffs

comment:59 Changed 6 years ago by andrew.mathas

OK, the script now uses difflib to compare the input and output files so hopefully this will fix the solaris problem and potential issues with other operating systems.

The tests pass me for me on a mac.

sage -t ../tests/cmdline.py
    [204 tests, 48.07 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 48.1 seconds
    cpu time: 0.2 seconds
    cumulative wall time: 48.1 seconds

A.

Last edited 6 years ago by andrew.mathas (previous) (diff)

comment:60 Changed 6 years ago by andrew.mathas

  • Status changed from needs_work to needs_review

comment:61 follow-up: Changed 6 years ago by tscrim

My output on Linux (Ubuntu) matches the output on Solaris. In other words, I also get the doctest failure noted above.

comment:62 Changed 6 years ago by tscrim

Actually, since it does diff properly, should we:

  • mark the doctest as # random,
  • look at the beginning and ending with a ... inbetween since it matches there?
Last edited 6 years ago by tscrim (previous) (diff)

comment:63 in reply to: ↑ 61 Changed 6 years ago by andrew.mathas

Replying to tscrim:

My output on Linux (Ubuntu) matches the output on Solaris. In other words, I also get the doctest failure noted above.

Hi Travis,

Sorry, this is my mistake: previously when you tested the script it worked, so the problem you're seeing is not because ubuntu's diff returns the same output as solaris. Rather that when I tested the the new patch I thought that I had the full patch applied but I was in the wrong branch where the patch was not applied. Sorry.

The new output, I hope, should be independent of the system diff as it uses difflib, which is a python module for generating diffs. In any case the new diff is closer to that returned by solaris than the previous universal diff.

With the patch applied, this time, I get the following:

sage -t cmdline.py
    [213 tests, 52.19 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 52.3 seconds
    cpu time: 0.2 seconds
    cumulative wall time: 52.2 seconds

The new version of trac_10589--doctests_for_fixdoctests-am.patch​ is now on trac.

Andrew

Changed 6 years ago by andrew.mathas

Makes fixdoctests use difflib to generate diffs

comment:64 Changed 6 years ago by tscrim

  • Status changed from needs_review to positive_review

Okay the new patch works for me on linux. Thanks Andrew.

comment:65 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.13.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:66 Changed 4 years ago by chapoton

  • Description modified (diff)
Note: See TracTickets for help on using tickets.