makerequest issues
Several things: 1) there is a "makerequest" function in both Testing/makerequest.py and Testing/ZopeTestCase/utils.py. They are subtly different. Is there a deliberate reason for this? I notice that the one in utils.py lacks the recent fix for zope 3 views, and the one in makerequest.py lacks an ACTUAL_URL. 2) re. the bug which I just posted at http://www.zope.org/Collectors/Zope/2057 the issue is that if you have something that inherits Traversable, and wrap it with makerequest(), its getPhysicalPath() method is broken, because getPhysicalPath() tries to call getPhysicalPath() on all non-None aq_parents, and an HTTPRequest isn't Traversable. There are two possible fixes to that bug: I could hack around it in makerequest() by adding a fake getPhysicalPath(). I've uploaded a patch to the collector that does this. - or - I could change the conditional in Traversable.getPhysicalPath to look like so: def getPhysicalPath(self): ... p = aq_parent(aq_inner(self)) if p is not None and getattr(p, 'getPhysicalPath', None) is not None: path = p.getPhysicalPath() + path ... I was reluctant to do the latter because of two worries: a) I figured it's called all over the place and even slightly adding to its workload might negatively impact performance; the attached timeit script shows about a 15% slowdown in the method, however, I've not been able to measure any difference when hitting a default plone front page with ab. b) I don't know if there is code that relies on the current behavior of getPhysicalPath(), i.e. raise an AttributeError if p is something other than None but still lacks getPhysicalPath. I can't imagine a use case, but who knows... Opinions? -- Paul Winkler http://www.slinkp.com
One small correction: On Fri, Mar 31, 2006 at 12:31:55PM -0500, Paul Winkler wrote:
... I notice that the makerequest in ZopeTestCase/utils.py lacks the recent fix for zope 3 views
No, it has that fix; I accidentally looked at it on a 2.9.1 tarball rather than the trunk or 2.9 branch. The rest of the questions still stand. -- Paul Winkler http://www.slinkp.com
I don't think makerequest is intended for wrapping anything but the root application object. Putting the RequestContainer on arbitrary objects doesn't feel right and certainly isn't how Zope does it, i.e. you get a test fixture that doesn't reflect "reality". NotABug/WontFix ;-) Stefan On 31. Mär 2006, at 19:31, Paul Winkler wrote:
2) re. the bug which I just posted at http://www.zope.org/Collectors/Zope/2057 the issue is that if you have something that inherits Traversable, and wrap it with makerequest(), its getPhysicalPath() method is broken, because getPhysicalPath() tries to call getPhysicalPath() on all non-None aq_parents, and an HTTPRequest isn't Traversable.
-- Anything that happens, happens. --Douglas Adams
On Sat, Apr 01, 2006 at 01:07:35PM +0200, Stefan H. Holek wrote:
I don't think makerequest is intended for wrapping anything but the root application object. Putting the RequestContainer on arbitrary objects doesn't feel right and certainly isn't how Zope does it, i.e. you get a test fixture that doesn't reflect "reality".
NotABug/WontFix ;-)
Hmmm, but unit tests very often don't reflect "reality" - deliberately! Because "reality" is Too Much Stuff. Any other opinions on this? Do we really need to require an App at the root any time we want to acquire REQUEST? That seems kind of arbitrary to me when any acquisition-enabled object would do. I've inherited a test suite that has a lot of stuff like: parent = makerequest(Folder()) ... and I don't see anything wrong with that in principle, but if the concensus is that Stefan's right, I'll bow to that. -- Paul Winkler http://www.slinkp.com
On 4. Apr 2006, at 16:53, Paul Winkler wrote:
Hmmm, but unit tests very often don't reflect "reality" - deliberately! Because "reality" is Too Much Stuff.
True enough.
Any other opinions on this? Do we really need to require an App at the root any time we want to acquire REQUEST? That seems kind of arbitrary to me when any acquisition-enabled object would do. I've inherited a test suite that has a lot of stuff like:
parent = makerequest(Folder())
... and I don't see anything wrong with that in principle, but if the concensus is that Stefan's right, I'll bow to that.
This looks fine to me because the world ends at parent. Your earlier example wrapped an object that was in the middle of an acquisition chain (IIRC), which I am not sure I like. You are safe to wrap the top-most object, be it a true "app" or not. Stefan -- Anything that happens, happens. --Douglas Adams
On Tue, Apr 04, 2006 at 08:09:05PM +0200, Stefan H. Holek wrote:
This looks fine to me because the world ends at parent. Your earlier example wrapped an object that was in the middle of an acquisition chain (IIRC),
no, I think you invented that :)
which I am not sure I like. You are safe to wrap the top-most object, be it a true "app" or not.
OK. In that case, 2057 really *is* a bug. http://www.zope.org/Collectors/Zope/2057 -- Paul Winkler http://www.slinkp.com
On 4. Apr 2006, at 22:08, Paul Winkler wrote:
On Tue, Apr 04, 2006 at 08:09:05PM +0200, Stefan H. Holek wrote:
This looks fine to me because the world ends at parent. Your earlier example wrapped an object that was in the middle of an acquisition chain (IIRC),
no, I think you invented that :)
Ok, I retract ;-)
which I am not sure I like. You are safe to wrap the top-most object, be it a true "app" or not.
OK. In that case, 2057 really *is* a bug. http://www.zope.org/Collectors/Zope/2057
Well, thing is that getPhysicalPath of Traversable looks like this: def getPhysicalPath(self): path = (self.getId(),) p = aq_parent(aq_inner(self)) if p is not None: path = p.getPhysicalPath() + path return path whereas getPhysicalPath of Application looks like this: def getPhysicalPath(self): # We're at the base of the path. return ('',) As you can see Application is designed to terminate the lookup recursion. With "safe" I meant with regard to acquiring the REQUEST, granted getPhysicalPath breaks (time for another half-retraction it seems). Regarding your patch, why do you use lambda: () and not lambda: ('',)? Stefan -- Anything that happens, happens. --Douglas Adams
On Wed, Apr 05, 2006 at 11:48:31AM +0200, Stefan H. Holek wrote:
Regarding your patch, why do you use lambda: () and not lambda: ('',)?
Because that would change the path of the wrapped object, which is not a desirable side effect. Here's the behavior with my patch:
from Testing.makerequest import makerequest from OFS.SimpleItem import SimpleItem item = SimpleItem(id='blah') # Wrapping an object with makerequest should not change its path. makerequest(app).getPhysicalPath() == app.getPhysicalPath() True makerequest(item).getPhysicalPath() == item.getPhysicalPath() True makerequest(item).getPhysicalPath() ('',)
Now here's the behavior with your suggestion:
from Testing.makerequest import makerequest from OFS.SimpleItem import SimpleItem item = SimpleItem(id='blah') makerequest(app).getPhysicalPath() == app.getPhysicalPath() True makerequest(item).getPhysicalPath() == item.getPhysicalPath() False item.getPhysicalPath() ('',) makerequest(item).getPhysicalPath() # uh-oh ('', '')
-- Paul Winkler http://www.slinkp.com
Out of curiosity, you know that SimpleItem does not have a constructor? You just inherit the one from ExtensionClass.Base, which accepts everything and subsequently ignores it. So your item will have an empty id. This *may* be what you want so your physical path starts with '' instead of 'blah', but I'm not sure... On 5. Apr 2006, at 17:21, Paul Winkler wrote:
item = SimpleItem(id='blah')
-- Anything that happens, happens. --Douglas Adams
On Wed, Apr 05, 2006 at 11:23:40PM +0200, Stefan H. Holek wrote:
Out of curiosity, you know that SimpleItem does not have a constructor? You just inherit the one from ExtensionClass.Base, which accepts everything and subsequently ignores it. So your item will have an empty id. This *may* be what you want so your physical path starts with '' instead of 'blah', but I'm not sure...
I didn't know that. Thanks. -- Paul Winkler http://www.slinkp.com
Stefan, do you have any insight into my first question? I wrote:
1) there is a "makerequest" function in both Testing/makerequest.py and Testing/ZopeTestCase/utils.py. They are subtly different. Is there a deliberate reason for this? I notice that ... (snip) the one in makerequest.py lacks an ACTUAL_URL.
If there's no reason for the implementations to differ, I can delete one definition and just import the other in its place. -- Paul Winkler http://www.slinkp.com
The one in ZopeTestCase.utils is also meant to play with startZServer (same module). I agree that the one in Testing.makerequest could probably gain ACTUAL_URL, and maybe even the request._steps hack to make URL1 and friends available... However, I have not seen these URL vars used anywhere but templates and DTML. ZTC adds them because it's used quite heavily for integration and functional testing(*). Also note that the values are faked, i.e. they are present but contain dummy values. In real-life they would be maintained by traversal, and no traversal takes place in unit tests. If you use ZTC you will automatically get an app object wrapped by utils.makerequest. If you don't use ZTC, I guess it depends on whether you think you will need ACTUAL_URL often enough to warrant adding it to Testing.makerequest. I don't think that's the case but YMMV. Stefan (*) To be honest they where likely added for Plone, now shoot me. On 4. Apr 2006, at 17:37, Paul Winkler wrote:
Stefan, do you have any insight into my first question? I wrote:
1) there is a "makerequest" function in both Testing/makerequest.py and Testing/ZopeTestCase/utils.py. They are subtly different. Is there a deliberate reason for this? I notice that ... (snip) the one in makerequest.py lacks an ACTUAL_URL.
If there's no reason for the implementations to differ, I can delete one definition and just import the other in its place.
-- Anything that happens, happens. --Douglas Adams
participants (2)
-
Paul Winkler -
Stefan H. Holek