#9739: Handle duplicate file basenames when testing multiple files in parallel
-------------------------------+--------------------------------------------
Reporter: mpatel | Owner: mvngu
Type: defect | Status: needs_review
Priority: critical | Milestone: sage-4.7.1
Component: doctest | Keywords: doctest scripts
Work_issues: | Upstream: N/A
Reviewer: Robert Bradshaw | Author: Mitesh Patel, John Palmieri
Merged: | Dependencies:
-------------------------------+--------------------------------------------
Comment(by jhpalmieri):
Replying to [comment:65 leif]:
> > This does not change SAGE_TESTDIR to `tempfile.gettempdir()`: we store
timing information in this directory, so it should not be temporary.
>
> Who cares? ;-)
Well, someone might...
> The odd thing with that is that we then again produce potential race
conditions for the timing files. I think we'd have to use locking then
(which can also cause headaches), perhaps on a follow-up.
True, theoretically, but I've never heard of this coming up. I don't
think these files are open for very long, so race conditions should be
very rare. I agree it should be dealt with on a follow-up, if at all.
(Instead of locking, we could perhaps use a "try ... except" block, since
if two processes are trying to write to the same timing file, it's not a
disaster if we just completely discard one of those.)
> IMHO these files also shouldn't be "hidden", and could live in or below
`DOT_SAGE`
I thought about this when working on the most recent patch, but it was too
much work for too little gain to make it backwards compatible (if
`.ptest_timing...` exists, then read it, otherwise, look for
`ptest_timing...`, etc.). It could be done on another ticket.
> (if we have to use locking anyway). Moreover, they perhaps shouldn't
get lost if one "manually" sets `SAGE_TESTDIR` to e.g. `/tmp`, which seems
reasonable at least as long as we don't automatically use some presumably
fast filesystem for the really temporary files.
That's a good point.
> > The changes: [[BR]]
> > - the filename mangling uses the full path of the file rather than
`tempfile.mkstemp`; this should be good enough, especially since we work
in a directory with name determined by the pid and the hostname.
>
> This doesn't help when simultaneously testing the same file from one
`sage-ptest` instance (`SAGE_TEST_ITER`, `SAGE_TEST_GLOBAL_ITER`) if I'm
not wrong; we could mangle '''`sage-doctest`'s''' PID into the
''filename'', too, as I suggested above.
I can do that, or we can use `mkstemp` instead of or in addition to the
path. Opinions either way?
> > - there are now messages printed about the doctesting directory, and
then deleting it at the end.
>
> For readability, I'd move the `print` statements (''"Removing the
directory ..."'', which hopefully don't raise exceptions) out of the `try`
block.
Okay.
> Also, `sage-ptest` should know whether tests failed (or doctesting was
interrupted), in which case we don't have to issue a warning since keeping
the failed doctest files is intentional (and the left files should have
been mentioned in previous messages).
>
> So I wouldn't try to remove the directory if any doctests failed, unless
they were [all] interrupted by Ctrl-C (in which case `sage-doctest` should
immediately remove all temporary files belonging to the aborted doctest,
which it currently doesn't).
Perhaps it should, but that should be on another ticket. As Dave said
earlier, "a sub-optimal solution is a better temporary measure than a
complete industrial strength bullet-proof solution". I don't want to have
to deal with the inner workings of doctesting here any more than is
necessary, and I don't think this particular issue is necessary to deal
with for this ticket. I can add a comment to the code about this.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9739#comment:66>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
