#13928: Problematic file filter in skip() from sage-ptest
---------------------------------+------------------------------------------
Reporter: ncohen | Owner: mvngu
Type: defect | Status: needs_review
Priority: major | Milestone: sage-5.6
Component: doctest | Resolution:
Keywords: | Work issues:
Report Upstream: N/A | Reviewers:
Authors: Nathann Cohen | Merged in:
Dependencies: | Stopgaps:
---------------------------------+------------------------------------------
Comment (by jhpalmieri):
Replying to [comment:14 nbruin]:
> Perhaps there's a middle way that satisfies both leif and nathan: I can
see why filenames starting with a "." should be skipped during discovery
and recursion, but if a dotted filename gets explicitly included in the
pathname, the user is clearly asking for testing that file, so why not?
Sounds good to me.
> Anyway, your current patch is fragile and not an improvement (other than
that it apparently fixes your particular problem). Any time you require a
`realpath` you're probably doing something wrong. It's a very expensive
operation and it's fragile. You should really work with file paths as
presented to you. The user probably had a reason for presenting the path
to you in the way he/she did. Don't second-guess him/her. In fact, with
remounting and loop mounting, files might not ''have'' a canonical
`realpath` anyway, showing that relying on it is a logical flaw.
I agree about using the path as presented by the user. Note though that
`realpath` is used several other places in the file, for example line 308:
{{{
if os.path.realpath(appendstr).startswith(BUILD_DIR):
}}}
where `BUILD_DIR` was also defined using `realpath`. So if it's
problematic, other parts of the file need to be fixed (as does the
[http://docs.python.org/2/library/os.path.html?highlight=realpath#os.path.realpath
Python documentation], unless they're using the word "canonical" in a,
ahem, noncanonical way). Or they don't need to be fixed, and they might
produce false positives: some files will get tested which perhaps
shouldn't. But it's better to test too many than to skip some, I think:
false positives are better than false negatives.
Replying to [comment:15 leif]:
> Replying to [comment:13 ncohen]:
> > Then what's the problem with filtering the `/.` that appear in
SAGE_ROOT ?
>
> I'm not sure what you mean by that. Of course RCSs' directories can be
located anywhere, not just below `SAGE_ROOT`, and even for the latter
aren't necessarily [just] Mercurial's `.hg` directories.
I think he means, what's wrong with just filtering subdirectories of
`SAGE_ROOT` (and its subdirectories) which start with `.`? This would be
consistent with Nils' suggestion.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13928#comment:17>
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.
