[Zope-dev] Re: [Checkins] SVN: Zope/trunk/ Fixed collector 2057: Testing.makequest broke getPhysicalPath()
On Thu, Apr 06, 2006 at 06:01:30PM -0400, Tres Seaver wrote: > Paul Winkler wrote: > > I know who originally added those tests, > > That would be me. I was hoping you'd pop in :-) > I don't see anything wrong with using a non-Zope2-app > object for unit testing: in fact, I think it is *superior* practice. > Most tests don't need to have the whole shooting match of the Zope > startup dance done, and in fact will be better unit tests if they are > tested *outside* of that environment. (snip) > I think you will see lots of usage of the pattern in CMF and related > products: the RequestTest and SecurityRequestTest base classes from > CMFCore are used *everywhere*. The only caveat is that, if your tests > *needs* to call 'getPhysicalPath' and get a "realistic" value, you need > to ensure that the object used as the root does the Right Thing (TM). Interesting. I'd forgotten that RequestTest et al. used this stuff; I had another look and what gets wrapped in makerequest there is whatever you get when you do Zope2.DB.open().root()['Application']. Is that not a Zope2.app? tangent: I really wish I'd looked at tests/base/testcase.py sooner, because I wasted time rewriting our LogInterceptor to work with the logging package - I didn't realize that a) it was a copy-paste from CMFCore, and b) the logging update was already done for CMF 2 if not sooner. Doh! Copy-paste is evil. Oh well, I learned some things about the logging package. > I don't beleive that "must be a Zope2.app()" is a real part of the > contract of 'makerequest': "must be able to emulate one well enough for > the rest of the test to succeed" is. That's a good thing to note in the docstrings. I'll go do that. Also I'll remove the getPhysicalPath() hackery. I don't really agree with Stefan that it's dangerous, but certainly it smells bad :-) -- Paul Winkler http://www.slinkp.com ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: [Checkins] SVN: Zope/trunk/ Fixed collector 2057: Testing.makequest broke getPhysicalPath()
On Fri, Apr 07, 2006 at 02:35:57PM +0200, Florent Guillaume wrote:
> Paul Winkler wrote:
> >Using an Item or Folder as your root object for tests works fine except for
> >this one issue, so why not allow that?
> >My feeling is that setting up an app is unnecessary work when you
> >don't need one; for one thing, your test module needs to call
> >Zope2.startup() first; for another, afaict creating a Zope2.app is
> >a couple orders of magnitude slower than creating a SimpleItem.
> >
> >So maybe more people *should* use makerequest(NotAnApp) ;-)
>
> FWIW usually in my tests I rarely use makerequest.
>
> On the other hand, when testing things that use traversal, it's very common
> that I have to define a fake root object whose getPhysicalPath returns
> ('',). It's only a few line, and makes my tests self-contained and much
> easier to understand than relying on the magic of a testing framework
> library (which I tend to hate).
I'm of two minds here. Those "only a few lines" add up very quickly.
I hate wading through 50 lines of setup for every two lines of actually
testing something. On the other hand, it's annoying when the test
framework magic does something unexpected. You just can't win. :-(
Whipping up my own request-wrapped root object for reuse internally
seems like a good solution.
--
Paul Winkler
http://www.slinkp.com
___
Zope-Dev maillist - Zope-Dev@zope.org
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: [Checkins] SVN: Zope/trunk/ Fixed collector 2057: Testing.makequest broke getPhysicalPath()
Paul Winkler wrote:
Using an Item or Folder as your root object for tests works fine except for
this one issue, so why not allow that?
My feeling is that setting up an app is unnecessary work when you
don't need one; for one thing, your test module needs to call
Zope2.startup() first; for another, afaict creating a Zope2.app is
a couple orders of magnitude slower than creating a SimpleItem.
So maybe more people *should* use makerequest(NotAnApp) ;-)
FWIW usually in my tests I rarely use makerequest.
On the other hand, when testing things that use traversal, it's very common
that I have to define a fake root object whose getPhysicalPath returns
('',). It's only a few line, and makes my tests self-contained and much
easier to understand than relying on the magic of a testing framework
library (which I tend to hate).
Florent
--
Florent Guillaume, Nuxeo (Paris, France) Director of R&D
+33 1 40 33 71 59 http://nuxeo.com [EMAIL PROTECTED]
___
Zope-Dev maillist - Zope-Dev@zope.org
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: [Checkins] SVN: Zope/trunk/ Fixed collector 2057: Testing.makequest broke getPhysicalPath()
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Paul Winkler wrote: > On Thu, Apr 06, 2006 at 11:17:20AM +0200, Stefan H. Holek wrote: > >>For the record: I am still opposed to this change. It basically >>endows the request (as in self.REQUEST) with a getPhysicalPath >>method, and I have no idea what kind of side-effects this may have. > > > That's because there aren't any ;-) > > The only way you can ever hit REQUEST.getPhysicalPath is if > you specifically ask for it, which is obviously stupid; or if > the object you're wrapping doesn't terminate getPhysicalPath(). > > >>AFAICS your test suite is the only suite around that wants to request- >>wrap non-root objects. > > > I'd reprhase that as "... wants to request-wrap non-Zope2.app() > objects." > > FWIW, I didn't write it, and at the time those tests were added, > I believe they worked. This was using zope 2.8.something. > > I know who originally added those tests, That would be me. I don't see anything wrong with using a non-Zope2-app object for unit testing: in fact, I think it is *superior* practice. Most tests don't need to have the whole shooting match of the Zope startup dance done, and in fact will be better unit tests if they are tested *outside* of that environment. > I was hired several months later and discovered some apparent bit-rot in > our test suite, i.e. a number of failures and errors, and one of the > principal points of failure was the issue we're discussing. > I'm not sure, but I'm guessing that the calls to getPhysicalPath() in > our app were added later, and whoever did that must've punted on > figuring out the resulting test issues. I think you will see lots of usage of the pattern in CMF and related products: the RequestTest and SecurityRequestTest base classes from CMFCore are used *everywhere*. The only caveat is that, if your tests *needs* to call 'getPhysicalPath' and get a "realistic" value, you need to ensure that the object used as the root does the Right Thing (TM). > >>There's nothing wrong with using your own >>makerequest for your own test suite, if this is what you want. But I >>don't think your use-case warrants changing Zope. > > > Noted, and that's a reasonable position. Does nobody else care? I don't beleive that "must be a Zope2.app()" is a real part of the contract of 'makerequest': "must be able to emulate one well enough for the rest of the test to succeed" is. > Using an Item or Folder as your root object for tests works fine except for > this one issue, so why not allow that? > My feeling is that setting up an app is unnecessary work when you > don't need one; for one thing, your test module needs to call > Zope2.startup() first; for another, afaict creating a Zope2.app is > a couple orders of magnitude slower than creating a SimpleItem. > > So maybe more people *should* use makerequest(NotAnApp) ;-) +1. Tres. - -- === Tres Seaver +1 202-558-7113 [EMAIL PROTECTED] Palladion Software "Excellence by Design"http://palladion.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFENY+6+gerLs4ltQ4RAiB7AJ9s/Ntu6pkWN8nyiQ6ifNfCj7DgPwCgzg0g SwE3IdhmcFdEnDL5kIpIiYU= =26iW -END PGP SIGNATURE- ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: [Checkins] SVN: Zope/trunk/ Fixed collector 2057: Testing.makequest broke getPhysicalPath()
On Thu, Apr 06, 2006 at 11:17:20AM +0200, Stefan H. Holek wrote: > For the record: I am still opposed to this change. It basically > endows the request (as in self.REQUEST) with a getPhysicalPath > method, and I have no idea what kind of side-effects this may have. That's because there aren't any ;-) The only way you can ever hit REQUEST.getPhysicalPath is if you specifically ask for it, which is obviously stupid; or if the object you're wrapping doesn't terminate getPhysicalPath(). > AFAICS your test suite is the only suite around that wants to request- > wrap non-root objects. I'd reprhase that as "... wants to request-wrap non-Zope2.app() objects." FWIW, I didn't write it, and at the time those tests were added, I believe they worked. This was using zope 2.8.something. I know who originally added those tests, and he's pretty high profile in the community, so I wouldn't be surprised if there are many such tests around. I was hired several months later and discovered some apparent bit-rot in our test suite, i.e. a number of failures and errors, and one of the principal points of failure was the issue we're discussing. I'm not sure, but I'm guessing that the calls to getPhysicalPath() in our app were added later, and whoever did that must've punted on figuring out the resulting test issues. > There's nothing wrong with using your own > makerequest for your own test suite, if this is what you want. But I > don't think your use-case warrants changing Zope. Noted, and that's a reasonable position. Does nobody else care? Using an Item or Folder as your root object for tests works fine except for this one issue, so why not allow that? My feeling is that setting up an app is unnecessary work when you don't need one; for one thing, your test module needs to call Zope2.startup() first; for another, afaict creating a Zope2.app is a couple orders of magnitude slower than creating a SimpleItem. So maybe more people *should* use makerequest(NotAnApp) ;-) -- Paul Winkler http://www.slinkp.com ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: [Checkins] SVN: Zope/trunk/ Fixed collector 2057: Testing.makequest broke getPhysicalPath()
For the record: I am still opposed to this change. It basically
endows the request (as in self.REQUEST) with a getPhysicalPath
method, and I have no idea what kind of side-effects this may have.
AFAICS your test suite is the only suite around that wants to request-
wrap non-root objects. There's nothing wrong with using your own
makerequest for your own test suite, if this is what you want. But I
don't think your use-case warrants changing Zope.
Stefan
On 5. Apr 2006, at 22:51, Paul Winkler wrote:
Log message for revision 66581:
Fixed collector 2057: Testing.makequest broke getPhysicalPath()
when used on anything other than an app object.
Also added an 'environ' argument if you want to use something
other than os.environ, and added a couple trivial tweaks from
ZopeTestCase.makereqeust.
Added unit tests for this stuff.
Changed:
U Zope/trunk/doc/CHANGES.txt
U Zope/trunk/lib/python/Testing/makerequest.py
A Zope/trunk/lib/python/Testing/tests/
A Zope/trunk/lib/python/Testing/tests/__init__.py
A Zope/trunk/lib/python/Testing/tests/test_makerequest.py
-=-
Modified: Zope/trunk/doc/CHANGES.txt
===
--- Zope/trunk/doc/CHANGES.txt 2006-04-05 20:04:59 UTC (rev 66580)
+++ Zope/trunk/doc/CHANGES.txt 2006-04-05 20:51:21 UTC (rev 66581)
@@ -46,6 +46,9 @@
Features added
+ - Testing.makerequest: Added an 'environ' argument so
+clients can use mappings other than os.environ.
+
- Updated to Docutils 0.4.0
- reStructuredText: The default value for the 'stylesheet'
@@ -202,6 +205,10 @@
Bugs Fixed
+ - Collector #2057: Allow Testing.makerequest to work with
+ any acquisition-supporting root object, not just Zope2.app.
+Formerly, if you did that, getPhysicalPath() was broken.
+
- Collector #2051: Applied patch by Yoshinori Okuji to fix some
XML export/import problems, including tests for that feature.
Modified: Zope/trunk/lib/python/Testing/makerequest.py
===
--- Zope/trunk/lib/python/Testing/makerequest.py 2006-04-05
20:04:59 UTC (rev 66580)
+++ Zope/trunk/lib/python/Testing/makerequest.py 2006-04-05
20:51:21 UTC (rev 66581)
@@ -19,27 +19,50 @@
import makerequest
app = makerequest.makerequest(Zope2.app())
+You can optionally pass stdout to be used by the response,
+and an environ mapping to be used in the request.
+Defaults are sys.stdout and os.environ.
+
+If you don't want to start a zope app in your test, you can wrap
other
+objects, but they must support acquisition and you should only wrap
+your root object.
+
+
$Id$
"""
import os
-from os import environ
from sys import stdin, stdout
from ZPublisher.HTTPRequest import HTTPRequest
from ZPublisher.HTTPResponse import HTTPResponse
from ZPublisher.BaseRequest import RequestContainer
-def makerequest(app, stdout=stdout):
+def makerequest(app, stdout=stdout, environ=None):
resp = HTTPResponse(stdout=stdout)
-environ['SERVER_NAME']='foo'
-environ['SERVER_PORT']='80'
-environ['REQUEST_METHOD'] = 'GET'
+if environ is None:
+environ = os.environ
+environ.setdefault('SERVER_NAME', 'foo')
+environ.setdefault('SERVER_PORT', '80')
+environ.setdefault('REQUEST_METHOD', 'GET')
req = HTTPRequest(stdin, environ, resp)
-
+req._steps = ['noobject'] # Fake a published object.
+req['ACTUAL_URL'] = req.get('URL') # Zope 2.7.4
+
# set Zope3-style default skin so that the request is usable for
-# Zope3-style view look-ups
+# Zope3-style view look-ups.
from zope.app.publication.browser import setDefaultSkin
setDefaultSkin(req)
-return app.__of__(RequestContainer(REQUEST = req))
+requestcontainer = RequestContainer(REQUEST = req)
+# Workaround for collector 2057: ensure that we don't break
+# getPhysicalPath if app has that method.
+# We could instead fix Traversable.getPhysicalPath() to check for
+# existence of p.getPhysicalPath before calling it; but it's such
+# a commonly called method that I don't want to impact
performance
+# for something that AFAICT only affects makerequest() in
+# practice.
+if getattr(app, 'getPhysicalPath', None) is not None:
+requestcontainer.getPhysicalPath = lambda: ()
+
+return app.__of__(requestcontainer)
Added: Zope/trunk/lib/python/Testing/tests/__init__.py
===
--- Zope/trunk/lib/python/Testing/tests/__init__.py 2006-04-05
20:04:59 UTC (rev 66580)
+++ Zope/trunk/lib/python/Testing/tests/__init__.py 2006-04-05
20:51:21 UTC (rev 66581)
@@ -0,0 +1 @@
+#
Property changes on: Zope/trunk/lib/python/Testing/tests/__init__.py
___
Name: svn:keywords
+ "Author Date Revision"
Added: Zope/trunk/lib/python/Testing/tests/tes
