Handle duplicate file basenames when testing multiple files in parallel

[e14f4152-4982-4ace-8c95-73a0599b109b]

When we test

/path/to/foo.py,

sage-doctest writes

SAGE_TESTDIR/.doctest_foo.py,

runs the new file through python, and deletes it. This can cause collisions when we test in parallel multiple files with the same basename, e.g., __init__, all, misc, conf, constructor, morphism, index, tests, homset, element, twist, tutorial, sagetex, crystals, cartesian_product, template, ring, etc.

There's a similar problem with testing non-library files, which sage-doctest first effectively copies to SAGE_TESTDIR.

See sage-devel for background.

This ticket may help with some of the doctesting problems discussed on the Sage mailing lists. Related tickets: #9224, #9225, #9449.

---

There's now a meta-ticket for all doctest-related issues / tickets, #11337.

---

I don't know much about this, so don't feel able to work on it.

But for those that do, perhaps a sub-optimal solution is a better temporary measure than a complete industrial strength bullet-proof solution.

Normally I don't like "quick fixes" that don't do the job properly, but in this case it seems if something is not done, then the problem will remain open forever, as nothing has actually been done on it for 7 months.

Sometimes I feel that trac tickets get held up for unnecessary long periods due to a reviewer being over pedantic and wanting things outside the scope of the original ticket, which would be better put on another ticket.

(Dave)

---

Apply

• attachment: trac_9739.v3.patch, attachment: trac_9739-extra.patch to the scripts repository
• attachment: trac_9739-graphviz.patch to the Sage library

CC: @sagetrac-drkirkby @jhpalmieri @nexttime @robertwb @jdemeyer

Component: doctest coverage

Keywords: doctest scripts race condition unique filenames ptestlong -tp

Author: Mitesh Patel, John Palmieri

Reviewer: Robert Bradshaw, Leif Leonhardy

Merged: sage-4.7.2.alpha3

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

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:1

William Stein suggests:

Could we use the tempfile module instead of using SAGE_TESTDIR.  The
tempfile module makes files and directories by default that are unique
and are *designed* to live on a fast filesystem, which gets cleaned
regularly.

sage: import tempfile 

[e14f4152-4982-4ace-8c95-73a0599b109b]

Description changed:

--- 
+++ 
@@ -13,3 +13,5 @@
 There's a similar problem with testing non-library files, which `sage-doctest` first effectively copies to `SAGE_TESTDIR`.

 See [sage-devel](http://groups.google.com/group/sage-devel/browse_thread/thread/0239f712a39fce4a/367bfc0d83c0e9b8#367bfc0d83c0e9b8) for background.
+
+This ticket may help with some of the doctesting problems discussed on the Sage mailing lists.  Related tickets: #9224, #9225, #9449.

[e14f4152-4982-4ace-8c95-73a0599b109b]

Attachment: basenames_frequencies.txt

Frequency-sorted list of doctest file basenames (includes .py, .pyx, .pxi, .rst, .tex files). Not a patch.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:2

I've attached a list of ~1500 unique basenames for the ~2500 files we doctest (give or take a handful).

[e14f4152-4982-4ace-8c95-73a0599b109b]

Doctest with unique temporary files. Apply to scripts repo.

[e14f4152-4982-4ace-8c95-73a0599b109b]

Author: Mitesh Patel

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:4

Attachment: trac_9739-unique_doctest_names.patch.gz

I've attached a patch. I haven't modified sage-test, since we're likely to phase it out at #9224.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:5

Hmmm, I'm not sure if people will like substituting the filename with the whole test command everywhere. (I have a wide screen, but...)

Can we please stop creating "hidden" doctest files? To me that doesn't make sense at all; especially but not limited to files in ~/.sage/*/.

Does the "For whitespace errors, see ..." still work (i.e., print the correct filename)?

The notion of "absolute filenames relative to ..." is a bit weird... (But thanks for renaming abs(). :) )

The rest seems ok; I've yet only looked at the patch though.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:6

While printing the number of global and file iterations even if both are 1 isn't that useful, I'd like to see the current / appropriate timeout setting[s] printed in sage-ptest (i.e. SAGE_TIMEOUT for normal tests, and only SAGE_TIMEOUT_LONG when -long was given).

(In principle, files containing no examples marked # long time should IMHO not be tested with SAGE_TIMEOUT_LONG, but the normal timeout. In that case we should print both timeout settings, and perhaps also indicate with which a file is being tested.)

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:7

Replying to @nexttime:

Hmmm, I'm not sure if people will like substituting the filename with the whole test command everywhere. (I have a wide screen, but...)

Ok, sorry, hard to read. The behavoir seems to be the same as before...

[bac7d3ea-3f1b-4826-8464-f0b53d5e12d2]

comment:8

How should I best apply this:

drkirkby@hawk:~/2/sage-4.5.3.alpha2$ cd local/bin
drkirkby@hawk:~/2/sage-4.5.3.alpha2/local/bin$ hg qimport  https://github.com/sagemath/sage-prod/files/10650483/trac_9739-unique_doctest_names.patch.gz
adding trac_9739-unique_doctest_names.patch to series file
drkirkby@hawk:~/2/sage-4.5.3.alpha2/local/bin$ hg push
abort: repository /space/rlm/sage-4.1.rc1/local/bin not found!

Something, somewhere seems to be looking for some directory of what I assume is Robert Millers.

Dave

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:9

Replying to @sagetrac-drkirkby:

Something, somewhere seems to be looking for some directory of what I assume is Robert Millers.

What happens if you (re)move SAGE_LOCAL/bin/.hg/hgrc? The problem might be that when you're in SAGE_LOCAL/bin, you're invoking ./hg, instead of /usr/local/bin/hg.

[robertwb]

comment:11

Note that you don't want to create a race condition with two tests trying to create the same directory at the same time. Perhaps mangling "/" -> "." would be sufficient.

[e14f4152-4982-4ace-8c95-73a0599b109b]

Reviewer: Robert Bradshaw

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:12

Replying to @robertwb:

Note that you don't want to create a race condition with two tests trying to create the same directory at the same time. Perhaps mangling "/" -> "." would be sufficient.

Thanks, Robert. How about adding the process ID, too, or instead? I'd also like to reduce the chance of races when we run multiple sage -t(p) commands simultaneously with the same DOT_SAGE. Or are there other potential races under this directory, e.g., in DOT_SAGE/gap?

[bac7d3ea-3f1b-4826-8464-f0b53d5e12d2]

comment:14

Has there been any more thoughts on this?

I had a doctest failure today on my OpenSolaris machine hawk, which is almost certainly a result of this bug. This is using sage-4.6.rc0.

The following tests failed:

    sage -t  -long local/lib/python2.6/site-packages/sagenb-0.8.2-py2.6.egg/sagenb/notebook/misc.py # 0 doctests failed
    sage -t  -long devel/sage/sage/plot/plot.py # 5 doctests failed

The likely reason for the first of these failures can be seen if we look further up the log.

sage -t  -long local/lib/python2.6/site-packages/sagenb-0.8.2-py2.6.egg/sagenb/misc/misc.py
         [2.9 s]
sage -t  -long local/lib/python2.6/site-packages/sagenb-0.8.2-py2.6.egg/sagenb/notebook/misc.py
python: can't open file '/export/home/drkirkby/.sage//tmp/misc.py': [Errno 2] No such file or directory

         [0.2 s]

The log clearly shows a file misc.py being tested, followed by a second test with a different file also called misc.py. That file is then not found. It seems almost inevitable one test deleted the file needed by another test as they were both called misc.py and both tested around the same time.

Note this is the same machine on which the buildbot passed all doctests.

http://build.sagemath.org/sage/builders/hawk%20full/builds/8/steps/shell_6/logs/stdio

so it seems to be an intermittent problem. (I've also had all tests pass on this).

These doctests issues are really annoying, as one never knows if its a real Sage bug, or a doctest bug.

Dave

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:16

I'm planning to return to this soon, probably after 4.6 is out.

[jdemeyer]

comment:17

Replying to @qed777:

I'm planning to return to this soon, probably after 4.6 is out.

That would be great, because I have also hit this error quite a few times.

[jdemeyer]

Changed keywords from none to doctest scripts

[bac7d3ea-3f1b-4826-8464-f0b53d5e12d2]

comment:19

There is another problem, which could exist even if every file had a different name.

If one tests multiple instances of Sage serially, then since they both write to $HOME/.sage, failures can occur even if the file names of the doctests are unique to any one copy of Sage. They need to be unique for any number of instances of Sage. I think testing under $HOME/.sage is a bit silly myself - it would be better to test under the directory where Sage is installed.

I found that testing devel/sage/sage/libs/fplll/fplll.pyx (see #10195), would generate problems when I was testing two copies of Sage at the same time (slightly different versions). I don't know if this patch can handle that situation, but it would be good if it could.

Dave

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:20

Replying to @sagetrac-drkirkby:

There is another problem, which could exist even if every file had a different name.

If one tests multiple instances of Sage serially, then since they both write to $HOME/.sage, failures can occur even if the file names of the doctests are unique to any one copy of Sage.

Well this would definitely be a user error. You can always set DOT_SAGE or SAGE_TESTDIR (or whatever it is called) if you want to run multiple tests simultaneously in different shells, even in / with the same Sage installation.

They need to be unique for any number of instances of Sage. I think testing under $HOME/.sage is a bit silly myself - it would be better to test under the directory where Sage is installed.

Definitely not, since this wouldn't work for site installations, where users usually have no write permissions under SAGE_ROOT.

I don't know if this patch can handle that situation, but it would be good if it could.

One could use Sage's PID, user and machine parameters etc. to try to create unique directories, or generally create "random" directories with mktemp (1) or mkdtemp(), but I think this would be an overkill, since the user can itself do such by setting one of the above variables.

[bac7d3ea-3f1b-4826-8464-f0b53d5e12d2]

comment:21

Replying to @nexttime:

Replying to @sagetrac-drkirkby:

There is another problem, which could exist even if every file had a different name.

If one tests multiple instances of Sage serially, then since they both write to $HOME/.sage, failures can occur even if the file names of the doctests are unique to any one copy of Sage.

Well this would definitely be a user error. You can always set DOT_SAGE or SAGE_TESTDIR (or whatever it is called) if you want to run multiple tests simultaneously in different shells, even in / with the same Sage installation.

They need to be unique for any number of instances of Sage. I think testing under $HOME/.sage is a bit silly myself - it would be better to test under the directory where Sage is installed.

Definitely not, since this wouldn't work for site installations, where users usually have no write permissions under SAGE_ROOT.

I don't know if this patch can handle that situation, but it would be good if it could.

One could use Sage's PID, user and machine parameters etc. to try to create unique directories, or generally create "random" directories with mktemp (1) or mkdtemp(), but I think this would be an overkill, since the user can itself do such by setting one of the above variables.

I disagree. I don't think creating unique temporary files is overkill. It would be far less error prone. With a test you want to run many times, having a dozen copies of Sage around for test purposes is quite a sensible thing to do with multi-core machines. Havving to set DOT_SAGE for each is annoying, when a unique temporary file could be made.

Dave

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:22

Replying to @sagetrac-drkirkby:

I disagree. I don't think creating unique temporary files is overkill.

I didn't say that, but it IMHO suffices to have unique names in the namespace of a ptest run.

It would be far less error prone. With a test you want to run many times, having a dozen copies of Sage around for test purposes is quite a sensible thing to do with multi-core machines. Havving to set DOT_SAGE for each is annoying, when a unique temporary file could be made.

As I said, you can automatically set up unique test directories by setting one of the above variables e.g. based on the "login" shell's PID, one per terminal / shell.

If multiple machines share the same filesystem, simply add e.g. the hostname, too.

[robertwb]

comment:23

+1 to temp (per instance) directories. They would get cleaned up properly, and /tmp is almost always fast and local which is another plus.

(Really, we shouldn't have to be writing these files out at all...)

[bac7d3ea-3f1b-4826-8464-f0b53d5e12d2]

comment:24

Replying to @robertwb:

+1 to temp (per instance) directories. They would get cleaned up properly, and /tmp is almost always fast and local which is another plus.

(Really, we shouldn't have to be writing these files out at all...)

I'm not sure if you are agreeing with me or Leif there Robert - perhaps you can clarify.

I was going to suggest that we should be using /tmp, but I did not since I can see a disadvantage of it. NFS file systems have caused problems with doc tests failing, especially if they are mis-configured. As such, it would be better if a user tested Sage on the file system where it will be used. By using /tmp they might get a false sense of security. That said, using /tmp for temporary files has been the norm for years.

I personally think where reasonably practical, we should stop multiple instances of running Sage tests interfering with each other. Although Leif considers this a user error, it is one that a user might easily make.

But if it possible to avoid creating temporary files, then that should be done. But I would imagine that requires more changes than just adding a pid or hostname.

Dave

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:25

OT: My make ptestlong of Sage 4.6 on Ubuntu 9.04 x86 somehow went into an infinite loop...

This never happened before (in dozens of builds), any idea?

(I started the complete build with make ptestlong, build succeeded and building the documentation went normal, but now I'm meanwhile at the tenth doctest run! Checked this with grep Doctesting ptestlong.log.)

[robertwb]

comment:26

Replying to @sagetrac-drkirkby:

Replying to @robertwb:

+1 to temp (per instance) directories. They would get cleaned up properly, and /tmp is almost always fast and local which is another plus.

(Really, we shouldn't have to be writing these files out at all...)

I'm not sure if you are agreeing with me or Leif there Robert - perhaps you can clarify.

Agreeing with drkirkby, we should use temp directories.

I was going to suggest that we should be using /tmp, but I did not since I can see a disadvantage of it. NFS file systems have caused problems with doc tests failing, especially if they are mis-configured. As such, it would be better if a user tested Sage on the file system where it will be used. By using /tmp they might get a false sense of security. That said, using /tmp for temporary files has been the norm for years.

Don't know if mktemp is POSIX, but it's widely available. Of course from Python you always have http://docs.python.org/library/tempfile.html Both are much better than manually specifying /tmp.

I personally think where reasonably practical, we should stop multiple instances of running Sage tests interfering with each other. Although Leif considers this a user error, it is one that a user might easily make.

I don't consider it a user error, and I also don't like filling .sage with lots of junk.

But if it possible to avoid creating temporary files, then that should be done. But I would imagine that requires more changes than just adding a pid or hostname.

True, and this ticket has been opened for far too long.

• Robert

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:27

Replying to @nexttime:

OT: My make ptestlong of Sage 4.6 on Ubuntu 9.04 x86 somehow went into an infinite loop...

This never happened before (in dozens of builds), any idea?

(I started the complete build with make ptestlong, build succeeded and building the documentation went normal, but now I'm meanwhile at the tenth doctest run! Checked this with grep Doctesting ptestlong.log.)

Ouch. I just noticed I had set SAGE_TEST_GLOBAL_ITER=1000 earlier in that shell...

(But all tests passed; I then aborted it during the 14th run.)

[jdemeyer]

comment:28

What is the current status of this patch, do you consider it ready for review? If not, I'm willing to join in and help where needed.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:29

Replying to @jdemeyer:

What is the current status of this patch, do you consider it ready for review? If not, I'm willing to join in and help where needed.

It's not ready for review. I don't think I have the time to work on this in the next several days, so if you'd like to help, please do!

I think we can make a temporary directory for each run in sage-(p)test and pass it to sage-doctest, which should ensure the temporary files under the directory are unique. The existing doesn't quite yet do the latter, but it should be easy to modify it so it does (e.g., with full paths and/or pids, etc.).

[bac7d3ea-3f1b-4826-8464-f0b53d5e12d2]

comment:30

Using both a hostname and a PID should mean the filename is practically unique if people test on more than one computer using a shared drive. I don't think 'mktemp' will be unique on NFS shared drives, though the probability of a collision would then be very small indeed. But adding a hostname would reduce it even further.

But we need to be careful if using mktemp. Whilst many systems have it, the implementation is not the same on every system. I know Solaris works a bit different to Linux or OS X (I forget which). I know using something on Solaris with mktemp which would not work with Linux or OS X. (I forget which OS it was though). It seems HP-UX and Solaris differ too.

I would be very keen to use something that will work on AIX. There is a chance of IBM donating a quad core 4.5 GHz machine to the Sage project with AIX on it.

Anyway, whatever solution is used, I think it will be 1000x better than the current solution, but personally I'd like to see something that's unique to a machine and portable.

Dave

[jdemeyer]

comment:31

I'm still interested in working on this patch, but I don't expect it to be ready on time for 4.6.1.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:32

While we are at it, I have some more comments on sage-ptest I'll also post here with an inline patch:

diff --git a/sage-ptest b/sage-ptest
--- a/sage-ptest
+++ b/sage-ptest
@@ -81,12 +81,18 @@
     Returns true if the file should not be tested
     """
     if not os.path.exists(F):
+        # XXX IMHO this should never happen; in case it does, it's certainly
+        #     an error to be reported (either filesystem, or bad name specified
+        #     on the command line). -leif
         return True
     G = abspath(F)
     i = G.rfind(os.path.sep)
+    # XXX The following should IMHO be performed in populatefilelist():
+    #     (Currently, populatefilelist() only looks for "__nodoctest__".)
     if os.path.exists(os.path.join(G[:i], 'nodoctest.py')):
         printmutex.acquire()
         print "%s (skipping) -- nodoctest.py file in directory"%abs(F)
+        sys.stdout.flush()
         printmutex.release()
         return True
     filenm = os.path.split(F)[1]
@@ -95,6 +101,7 @@
         return True
     if G.find(os.path.join('doc', 'output')) != -1:
         return True
+    # XXX The following is (also/already) handled in populatefilelist():
     if not (os.path.splitext(F)[1] in ['.py', '.pyx', '.tex', '.pxi', '.sage', '.rst']):
         return True
     return False
@@ -115,6 +122,7 @@
     for i in range(0,numiteration):
         os.chdir(os.path.dirname(F))
         command = os.path.join(SAGE_ROOT, 'local', 'bin', 'sage-%s' % cmd)
+        # FIXME: Why call bash here? (Also, we use 'shell=True' below anyway.)
         s = 'bash -c "%s %s > %s" ' % (command, filestr, outfile.name)
         try:
             t = time.time()
@@ -161,10 +169,12 @@
         print sage_test_cmd(F[len(CUR)+1:])
     else:
         print abs(F)
+    sys.stdout.flush()
     if ol!="" and (not ol.isspace()):
         if (ol[len(ol)-1]=="\n"):
             ol=ol[0:len(ol)-1]
         print ol
+        sys.stdout.flush()
     time_dict[abs_sage_path(F)] = finished_time
     if XML_RESULTS:
         t = finished_time
@@ -192,6 +202,7 @@
         """.strip() % locals())
         f.close()
     print "\t [%.1f s]"%(finished_time)
+    sys.stdout.flush()

 def infiles_cmp(a,b):
     """
@@ -231,7 +242,14 @@
                 base, ext = os.path.splitext(F)
                 if not (ext in ['.sage', '.py', '.pyx', '.tex', '.pxi', '.rst']):
                     continue
-                elif '__nodoctest__' in files:
+                elif '__nodoctest__' in files: # XXX Shouldn't this be 'lfiles'?
+                    # Also, this test should IMHO be in the outer loop (1 level).
+                    # Furthermore, the current practice is to put "nodoctest.py"
+                    # files in the directories that should be skipped, not
+                    # "__nodoctest__". (I haven't found a single instance of the
+                    # latter in Sage 4.6.1.alpha3.)
+                    # "nodoctest.py" is handled in skip() (!), to also be fixed.
+                    # -leif
                     continue
                 appendstr = os.path.join(root,F)
                 if skip(appendstr):
@@ -252,6 +270,9 @@
     argv = [X for X in argv if X[0] != '-']

     try: 
+        # FIXME: Nice, but <NUMTHREADS> should immediately follow '-tp' etc.,
+        #        i.e., be the next argument. We might have file or directory
+        #        names that properly convert to an int...
         numthreads = int(argv[1])
         infiles = argv[2:]
     except ValueError: # can't convert first arg to an integer: arg was probably omitted

(This is against Sage 4.6.1.alpha3.)

The comments all refer to inconsistencies; the only actual change is flushing the output since it currently comes in bursts, which is IMHO odd for watching it. I don't think this measurably slows down doctesting...

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:33

Just for the record:

10458 also patches sage-ptest to support IPython/Sage-style line continuations in doctests ("....:" rather than only "...").

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:34

Replying to @nexttime:

Just for the record:

10458 also patches sage-ptest to support IPython/Sage-style line continuations in doctests ("....:" rather than only "...").

Ooops, sorry, #10458 patches sage-doctest, not sage-ptest.

[ghost]

Replying to @qed777:

When we test

/path/to/foo.py

sage-doctest writes

SAGE_TESTDIR/.doctest_foo.py

runs the new file through python, and deletes it. This can cause collisions when we test in parallel multiple files with the same basename, e.g., __init__, all, misc, conf, constructor, morphism, index, tests, homset, element, twist, tutorial, sagetex, crystals, cartesian_product, template, ring, etc.

There's a similar problem with testing non-library files, which sage-doctest first effectively copies to SAGE_TESTDIR.

See sage-devel for background.

This ticket may help with some of the doctesting problems discussed on the Sage mailing lists. Related tickets: #9224, #9225, #9449.

[ghost]

Changed keywords from doctest scripts to currency trading, forex analysis, forex trading, online forex trading

[ghost]

Description changed:

--- 
+++ 
@@ -1,17 +1,3 @@
-When we test
+![](http://www.forextradingcoach-simon.com/wp-content/uploads/2010/01/forex-trading.jpg)

-`/path/to/foo.py`
-
-`sage-doctest` writes
-
-`SAGE_TESTDIR/.doctest_foo.py`
-
-runs the new file through `python`, and deletes it.  This can cause
-collisions when we test in parallel multiple files with the same
-basename, e.g., `__init__`, `all`, `misc`, `conf`, `constructor`, `morphism`, `index`, `tests`, `homset`, `element`, `twist`, `tutorial`, `sagetex`, `crystals`, `cartesian_product`, `template`, `ring`, etc.
-
-There's a similar problem with testing non-library files, which `sage-doctest` first effectively copies to `SAGE_TESTDIR`.
-
-See [sage-devel](http://groups.google.com/group/sage-devel/browse_thread/thread/0239f712a39fce4a/367bfc0d83c0e9b8#367bfc0d83c0e9b8) for background.
-
-This ticket may help with some of the doctesting problems discussed on the Sage mailing lists.  Related tickets: #9224, #9225, #9449.
+If you are an expert on **[forex trading](http://www.forexinsider.co.uk)** you know already what it is to be a forex trader and the process that you have been in to is not easy but if you know how to handle tough tradings then it would be good for you because it will be who you are when you mature in this type of business.

[jasongrout]

Description changed:

--- 
+++ 
@@ -1,3 +1 @@
-![](http://www.forextradingcoach-simon.com/wp-content/uploads/2010/01/forex-trading.jpg)

-If you are an expert on **[forex trading](http://www.forexinsider.co.uk)** you know already what it is to be a forex trader and the process that you have been in to is not easy but if you know how to handle tough tradings then it would be good for you because it will be who you are when you mature in this type of business.

[jasongrout]

Changed keywords from currency trading, forex analysis, forex trading, online forex trading to doctest scripts

[jasongrout]

comment:37

Please ban forextrading11 for spam.

[7c09a680-e216-4024-bb8e-9bfd4aa7f313]

comment:38

Replying to @jasongrout:

Please ban forextrading11 for spam.

Done.

[bac7d3ea-3f1b-4826-8464-f0b53d5e12d2]

comment:40

Replying to @jdemeyer:

This is marked as a blocker for 4.7. but there's been no work on it for 8 weeks.

Dave

[bac7d3ea-3f1b-4826-8464-f0b53d5e12d2]

Description changed:

--- 
+++ 
@@ -1 +1,9 @@
+I don't know much about this, so don't feel able to work on it. 

+But for those that do, perhaps a sub-optimal solution is a better temporary measure than a complete industrial strength bullet-proof solution. 
+
+Normally I don't like "quick fixes" that don't do the job properly, but in this case it seems if something is not done, then the problem will remain open forever, as nothing has actually been done on it for 7 months. 
+
+Sometimes I feel that trac tickets get held up for unnecessary long periods due to a reviewer being over pedantic and wanting things outside the scope of the original ticket, which would be better put on another ticket. 
+
+

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:43

To me it would -- at least for the moment, or this ticket -- be totally sufficient to have unique filenames by mangling the relative path into the temporary filename.

If someone wants to test multiple Sage installations at the same time, he can simply set SAGE_TESTDIR to different locations in each shell.

Doctesting in (or below) $DOT_SAGE (which is usually $HOME/.sage/) is IMHO a bad idea anyway, not only because /tmp/ is much more likely to be [on] a local filesystem than /home/. (It is usually also faster and auto-cleaned.) Creating a unique temporary directory there shouldn't be a problem.

For the sake of "For whitespace errors see ...", failing files could still be copied elsewhere (to a perhaps more persistent location) at the end of the doctest process.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

Description changed:

--- 
+++ 
@@ -1,3 +1,25 @@
+When we test
+
+`/path/to/foo.py`,
+
+`sage-doctest` writes
+
+`SAGE_TESTDIR/.doctest_foo.py`,
+
+runs the new file through `python`, and deletes it.  This can cause collisions when we test in parallel multiple files with the same basename, e.g., `__init__`, `all`, `misc`, `conf`, `constructor`, `morphism`, `index`, `tests`, `homset`, `element`, `twist`, `tutorial`, `sagetex`, `crystals`, `cartesian_product`, `template`, `ring`, etc.
+
+There's a similar problem with testing non-library files, which `sage-doctest` first effectively copies to `SAGE_TESTDIR`.
+
+See [sage-devel](http://groups.google.com/group/sage-devel/browse_thread/thread/0239f712a39fce4a/367bfc0d83c0e9b8#367bfc0d83c0e9b8) for background.
+
+This ticket may help with some of the doctesting problems discussed on the Sage mailing lists.  Related tickets: #9224, #9225, #9449.
+
+---
+
+There's now a meta-ticket for all doctest-related issues / tickets, #11337.
+
+---
+
 I don't know much about this, so don't feel able to work on it. 

 But for those that do, perhaps a sub-optimal solution is a better temporary measure than a complete industrial strength bullet-proof solution. 
@@ -6,4 +28,4 @@

 Sometimes I feel that trac tickets get held up for unnecessary long periods due to a reviewer being over pedantic and wanting things outside the scope of the original ticket, which would be better put on another ticket. 

-
+(Dave)

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:44

I've restored the original description the spammer deleted.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:45

For the record, we already have the following in sage/misc/misc.py:

...
HOSTNAME = socket.gethostname().replace('-','_').replace('/','_').replace('\\','_')

LOCAL_IDENTIFIER = '%s.%s'%(HOSTNAME , os.getpid())
...
try:
    DOT_SAGE = os.environ['DOT_SAGE']
except KeyError:
    try:
        DOT_SAGE = '%s/.sage/'%os.environ['HOME']
    except KeyError:
        DOT_SAGE = '%s/.sage/'%SAGE_ROOT
...
if not os.path.exists(DOT_SAGE):
    os.makedirs(DOT_SAGE)

_mode = os.stat(DOT_SAGE)[stat.ST_MODE]
_desired_mode = 040700     # drwx------
if _mode != _desired_mode:
    print "Setting permissions of DOT_SAGE directory so only you can read and write it."
    # Change mode of DOT_SAGE.
    os.chmod(DOT_SAGE, _desired_mode)
...
SAGE_TMP='%s/temp/%s/%s/'%(DOT_SAGE, HOSTNAME, os.getpid())
if not os.path.exists(SAGE_TMP):
    try:
        os.makedirs(SAGE_TMP)
    except OSError, msg:
        print msg
        raise OSError, " ** Error trying to create the Sage tmp directory..."
...

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:46

... so we could do almost the same for / below SAGE_TESTDIR, perhaps creating a single directory name from HOSTNAME and the PID (avoiding further race conditions, provided $SAGE_TESTDIR already exists), putting all temporary files (names mangled as proposed, unique to each test instance) into that, i.e.

${SAGE_TESTDIR}/${hostname}-${pid}/doctest-relative__path__to__foo__foo.py

for a file ./relative/path/to/foo/foo.py.

[jhpalmieri]

comment:47

Can we just use Python's tempfile module, for example mkstemp? This should produce a temporary file safely, avoiding race conditions. We can have the name end with the file being tested, including its path.

(I think we should do the same thing for SAGE_TMP in misc.py, but that's for another ticket.)