[issue1259] string find and rfind methods give a TypeError that is misleading
New submission from
Robert Collins
:
Python 2.5.1 (r251:54863, May 2 2007, 16:56:35)
[GCC 4.1.2 (Ubuntu 4.1.2-0ubuntu4)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> 'asd'.find('s', None, None)
Traceback (most recent call last):
File "", line 1, in
TypeError: slice indices must be integers or None or have an __index__
method
>>> 'asd'.rfind('s', None, None)
Traceback (most recent call last):
File "", line 1, in
TypeError: slice indices must be integers or None or have an __index__
method
>>> # Note that this works, at the price of a memory copy,
>>> # and on large strings that is undesirable.
>>> 'asd'[None:None].find('s')
1
>>> 'asd'[None:None].rfind('s')
1
>>>
--
messages: 56332
nosy: rbcollins
severity: normal
status: open
title: string find and rfind methods give a TypeError that is misleading
versions: Python 2.5
__
Tracker <[EMAIL PROTECTED]>
<http://bugs.python.org/issue1259>
__
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1259] string find and rfind methods give a TypeError that is misleading
Robert Collins added the comment: > The error message is wrong: it's a TypeError, but the message should say > something like... > > TypeError: slice indices must be integers or have an __index__ method This would be a false message, as, as my report demonstrated, slice indices *can* be None. And there is a very good reason for accepting None in the second parameter in slice notation as there is no value for '-0' to represent the end of the region being sliced, and None meaning 'default' fills that need most effectively. Surely the right fix is as Barry noted, to handle None correctly within this function? __ Tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue1259> __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
New submission from Robert Collins : This affects the python implementation of RLock only. If a signal occurs during RLock.acquire() or release() and then operates on the same lock to acquire() or release() it, process hangs or assertions can be triggered. The attached test script demonstrates the issue on Python 2.6 and 3.2, and code inspection suggests this is still valid for 2.7 and 3.4. To use it, run the script and then 'kill -SIGUSR2' the process id it prints out. Keep sending SIGUSR2 until you get bored or: - a traceback occurs - the process stops printing out '.'s We found this debugging application server hangs during log rotation (https://bugs.launchpad.net/launchpad/+bug/861742) where if the thread that the signal is received in was about to log a message the deadlock case would crop up from time to time. Of course, this wasn't ever safe code, and we're changing it (to have the signal handler merely set a integer flag that the logging handler can consult without locking). However, I wanted to raise this upstream, and I note per issue #3618 that the 3.x IO module apparently includes a minimal version of the Python RLock implementation (or just uses RLock itself). Either way this bug probably exists for applications simply doing IO, either when there is no C RLock implementation available, or all the time (depending on exactly what the IO module is doing nowadays). -- components: Library (Lib) files: hang.py messages: 150477 nosy: rbcollins priority: normal severity: normal status: open title: python RLock implementation unsafe with signals versions: Python 2.7, Python 3.4 Added file: http://bugs.python.org/file24126/hang.py ___ Python tracker <http://bugs.python.org/issue13697> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Robert Collins added the comment: I'm not sure it is sensibly implementable in pure python: the semantics of signal handling (AIUI) are that the vm is interrupted, sets a flag to say 'when the GIL is released or the next bytecode interpretation happens, please process signal X' [the former for C extensions, the latter for regular code], and then the OS level signal handler returns. The current C or bytecode execution completes and then we dispatch to the python signal handler. Now, what we would want here is that attempts to acquire/release the RLock in a signal handler would behave as though the RLock.acquire/release methods were atomic. The general way to enforce such atomicity is via a lock or mutex. Now, because the call stack looks like this: thread.acquire() thread.acquire() The other acquire, if it already holds this supposed mutex, would block the python signal handler acquire call from obtaining it; The inner acquire would have to error *whether or not a timeout was passed*, because the non-signal-code won't progress and complete until the python signal handler code returns. One could, in principle, use stack inspection to determine how far through a nested acquire/release the outer code is, but that seems, uhm, pathological to me :). The crux of the problem is detecting whether the non-reentrant thread.lock is held because (a) another thread holds it or (b) this thread holds it. If we can inspect its internals, we could determine that this thread holds it - and thus we must be reentered into the acquire/release methods. With that tricky thing established, we could look at the remaining logic to make sure that each line is either atomic or conditional on reentrancy. Simpler I think to require CRLocks always, and provide a portable C implementation which will always complete before returning to execution in the calling thread, avoiding this issue entirely. -- ___ Python tracker <http://bugs.python.org/issue13697> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Robert Collins added the comment: Normally I advocate very strongly for Python implementation of C accelerated modules, but when the two implementations are not equivalent, having a simpler Python one around does not help anyone (not users, other language implementors etc). True reentrancy is possible but quite hard to achieve. So, FWIW, +1 on just having a C implementation. The dedicated signal thread sounds useful too - it would ease the issues for folk using other parts of the stdlib, and would in principle permit a pure-python RLock to hang around. -- ___ Python tracker <http://bugs.python.org/issue13697> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10440] support RUSAGE_THREAD as a constant in the resource module
New submission from Robert Collins : RUSAGE_THREAD (since Linux 2.6.26) Return resource usage statistics for the calling thread. This is very handy for multi threaded apps in determining runtime in a thread, page faults from the thread etc. -- messages: 121336 nosy: rbcollins priority: normal severity: normal status: open title: support RUSAGE_THREAD as a constant in the resource module type: feature request ___ Python tracker <http://bugs.python.org/issue10440> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10440] support RUSAGE_THREAD as a constant in the resource module
Changes by Robert Collins : -- keywords: +patch Added file: http://bugs.python.org/file19624/rusage-thread.patch ___ Python tracker <http://bugs.python.org/issue10440> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7501] python -m unittest path_to_suite_function errors
Robert Collins added the comment: Its a common convention in zope.testing, trial, testtools, bzr, ... -- ___ Python tracker <http://bugs.python.org/issue7501> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24653] Mock.assert_has_calls([]) is surprising for users
Robert Collins added the comment: I'm reopening this because I don't agree. I opened the bug because we have evidence that users find the current documentation confusing. Saying that its not confusing to us doesn't fix the confusion. Why should we mention the special case of an empty set? Because its the only special case. A simple single sentence: "Note: to assert that no calls were made see `assert_not_called`" would probably both cover the special case and direct users to the right place for that use case. -- resolution: wont fix -> status: closed -> open ___ Python tracker <https://bugs.python.org/issue24653> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue34125] Profiling depends on whether **kwargs is given
Robert Collins added the comment: New changeset b892d3ea468101d35e2fb081002fa693bd86eca9 by Robert Collins (Jeroen Demeyer) in branch 'master': bpo-36994: add test for profiling method_descriptor with **kwargs (GH-13461) https://github.com/python/cpython/commit/b892d3ea468101d35e2fb081002fa693bd86eca9 -- nosy: +rbcollins ___ Python tracker <https://bugs.python.org/issue34125> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue36994] Missing tests for CALL_FUNCTION_EX opcode profiling method_descriptor
Robert Collins added the comment: New changeset b892d3ea468101d35e2fb081002fa693bd86eca9 by Robert Collins (Jeroen Demeyer) in branch 'master': bpo-36994: add test for profiling method_descriptor with **kwargs (GH-13461) https://github.com/python/cpython/commit/b892d3ea468101d35e2fb081002fa693bd86eca9 -- nosy: +rbcollins ___ Python tracker <https://bugs.python.org/issue36994> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue36656] Please add race-free os.link and os.symlink wrapper / helper
Robert Collins added the comment: I'd like to add a few notes; please do consider Windows interactions here - Windows does not have the same model for inode replacement that Posix has. Secondly, I note that the thread model discussed here hasn't been particular well articulated. In particular the vagaries of directories with the sticky bit set are quite different to those where attacker and victim share group permissions : TOCTTOU attacks do imply that post-atomic operation attacks will be possible, and I disagree with the analysis by Serhiy suggesting that they are necessarily so. However I also agree with Toshio that the os module is not the right place to provide a more secure API: we have to have the basic primitive exposed to Python code somewhere, otherwise the higher level APIs such as you'd like to use are not creatable. My suggestion is that you put a module up on PyPI that provides the secure behaviour necessary, get some feedback on that, get some cross-platform experience, and then, if desired, propose it for inclusion in shutil or similar in the stdlib. I'm also going to retitle this - because actually, os.link and os.symlink aren't racy *per se* on Posix - its how they are used that is racy: and the very fact that a secure variant can be written (however ugly) is demonstration of that. -- nosy: +rbcollins title: Race conditions due to os.link and os.symlink POSIX specification -> Please add race-free os.link and os.symlink wrapper / helper ___ Python tracker <https://bugs.python.org/issue36656> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue36656] Please add race-free os.link and os.symlink wrapper / helper
Robert Collins added the comment: Thank you @Eryk -- ___ Python tracker <https://bugs.python.org/issue36656> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19645] decouple unittest assertions from the TestCase class
Robert Collins added the comment: Sorry for the slow reply here; There are API breaks involved in any decoupling that involves the exception raising because of the failureException attribute. Something with signalling that can be adapted by other test suites etc might have merit, but I think we are lacking a clear use case for doing this to the existing exceptions. Setting up a way for new things to be more easily used by users of other test frameworks is quite attractive; perhaps just writing them as separate functions with an adapter to failureException would be sufficient. -- versions: +Python 3.9 -Python 3.5 ___ Python tracker <https://bugs.python.org/issue19645> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37262] Make unittest assertions staticmethods/classmethods
Robert Collins added the comment: I think this is strictly redundant with that other ticket and I'm going to close it. That said, they need access to self.failureException. https://docs.python.org/3/library/unittest.html#unittest.TestCase.failureException -- stage: -> resolved status: open -> closed ___ Python tracker <https://bugs.python.org/issue37262> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19645] decouple unittest assertions from the TestCase class
Robert Collins added the comment: Right now that attribute could be set by each test separately, or even varied within a test. TBH I'm not sure that the attribute really should be supported; perhaps thinking about breaking the API is worth doing. But - what are we solving for here. The OP here seems interested in using the assertion like things entirely outside of a test context. What would a nice clean API for that be? (Yes I like matchers, but put that aside - if the APIs aren't close enough, lets make sure we do a good job for each audience rather than a compromise..) -- ___ Python tracker <https://bugs.python.org/issue19645> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19645] decouple unittest assertions from the TestCase class
Robert Collins added the comment: Ok so design wise - there is state on the TestCase that influences assertions; in potentially two ways. The first way is formatting - the amount of detail shown in long list comparisons etc. The second way I don't think we have at the moment, but potentially it could influence the fidelity of comparisons for NearlyEquals and the like - generally though we tend to pass those in as parameters. So just ripping everything off into standalone functions loses the home for that state. It either becomes global (ugh), or a new object that isn't a test case but is basically the same magic object that has to be known is carried around, or we find some other way of delegating the formatting choice and controls. hamcrest has some prior art in this space, and testtools experimented with that too. wordpress has managed to naff up the formatting on my old blog post about this https://rbtcollins.wordpress.com/2010/05/10/maintainable-pyunit-test-suites/ and https://testtools.readthedocs.io/en/latest/for-test-authors.html#matchers Its been on my TODO for a very long time to put together a PEP for adding matchers to the stdlib; I find the full system we did in testtools very useful because it can represent everything from a trivial in-memory string error through to a disk image for a broken postgresql database, without running out of memory or generating mojibake but if we wanted to do something smaller that didn't prejuidice extensions like testtools still doing more that would be fine too. The core idea of matchers is that rather than a standalone function f() -> nil/raise, you build a callable object f() -> Option(Mismatch), and a Mismatch can be shown to users, combined with other mismatches to form composites or sequences and so forth. So this would give room for the state around object formatting and the like too. -- ___ Python tracker <https://bugs.python.org/issue19645> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19645] decouple unittest assertions from the TestCase class
Change by Robert Collins : -- versions: +Python 3.9 -Python 3.5 ___ Python tracker <https://bugs.python.org/issue19645> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19645] decouple unittest assertions from the TestCase class
Robert Collins added the comment: Oh, I didn't mean to imply that these are the only options I'd support - just that these are the things I've thought through and that I think will all work well... I'm sure there are more options available ;) -- ___ Python tracker <https://bugs.python.org/issue19645> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue30221] Deprecate assertNotAlmostEqual
Robert Collins added the comment: We've now spent more time debating the deprecation that we would have saved if it had been deprecated. Deprecations cost user good will. Whilst I very much want to delete all assertions in favour of matchers, which would allow composed symmetry rather than the manual symmetry we have today. Like michael I don't see any benefit in getting rid of it either. I don't believe it will make the API substantially easier to learn: the lack of symmetry increases special cases, so it will IMO make it harder to learn. Unless you've new arguments to make about this, I think the ticket can be closed WONTFIX. -- ___ Python tracker <http://bugs.python.org/issue30221> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue26903] ProcessPoolExecutor(max_workers=64) crashes on Windows
Robert Collins added the comment: This is now showing up in end user tools like black: https://github.com/ambv/black/issues/564 -- nosy: +rbcollins versions: +Python 3.7, Python 3.8, Python 3.9 ___ Python tracker <https://bugs.python.org/issue26903> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33021] Some fstat() calls do not release the GIL, possibly hanging all threads
Robert Collins added the comment: Re: backporting this. I think backporting to 3.6/3.7 makes a lot of sense - bugfix and prerelease respectively. For 2.7, this bug has been around for ages, the patch is small, and I have no objection - but the RM has already said no, so I guess not happening :). That said, if I was analyzing this from scratch I'd look at the pypi download stats to assess what footprint 2.7 still has, and whether taking this fix there would make objective sense. While it is a genuine bug - and a nice catch - I have to say that running any Python with mmapped data off of NFS is a supremely bad idea :). -- nosy: +rbcollins ___ Python tracker <https://bugs.python.org/issue33021> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32419] Add unittest support for pyc projects
Robert Collins added the comment: Oh, and why look for __init__ - in part, because it predates namespace packages, but also because unlike regular imports unittest will do negative things like reading the entire filesystem otherwise. -- ___ Python tracker <https://bugs.python.org/issue32419> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32419] Add unittest support for pyc projects
Robert Collins added the comment: Whats the use for *unittest* - a tool to help folk develop - to run a test which is only present in sourceless form? -- ___ Python tracker <https://bugs.python.org/issue32419> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue27071] unittest.TestCase.assertCountEqual is a very misleading name
Robert Collins added the comment: So, I think we're in a local minima here. As I said earlier, neither the old nor new names really grab me, and I think everyone has aknowledged that the new name is -at best- confusing. We don't need *any more discussion* about that. The question is how to fix it without yet more rounds of 'well this is confusing so lets change it again'. I want to suggest some basic criteria for a dramatically better name, just to unblock this. - follow the existing assert naming convention - be short enough to be comfortably usable - pass some user testing: do some lightning talks. Or ask on twitter for folk to tell you what the proposed name does without any hints. - follow up here with how you tested and the name you've chosen based on that testing. - we'll then do a straw poll amongst a few committers - me and michael at a minimum, and advise on go/nogo. Once thats done I'll happily review a patch that adds a new, better name as an alias, and doesn't deprecate the existing one. I'm leaving the bug in rejected status, because at this point there is nothing any committers seem to have appetite/energy to do: same as per https://bugs.python.org/msg266244 -- ___ Python tracker <https://bugs.python.org/issue27071> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24412] setUpClass equivalent for addCleanup
Robert Collins added the comment: Serhiy, thats not a design flaw, its a feature. in a class hierarchy, setup and teardown ordering is undefined: implementors can choose whatever order they want to execute in. e.g. class A(TestCase): def setUp(self): super().setUp() acquire1() class B(A): def setUp(self): acquire2() super().setUp() class C(B): def setUp(self): super().setUp() acquire3() will acquire 2, then 1, then 3. tearDown() is similarly ill defined. Secondly, consider class Example(TestCase): def setUp(self): self._1 = acquire() self.addCleanUp(acquire()) self._3 = acquire() def tearDown(self): self._3.cleanUp() self._1.cleanUp() super().tearDown() As such, there is no ordering of cleanup + teardown that is 'right' in all cases. The question then becomes, which ordering *most facilitates* migration from tearDown to cleanup. The allowable orders are: - with a more complex implementation, per-class (no) - before tearDown starts - after tearDown finishes The common case tearDown tends to look like this: def tearDown(self): super().tearDown() so, by placing doCleanup after tearDown, we make it possible for base classes in a test hierarchy to migrate to cleanups without breaking compatibility with child classes. The cost, as you note is that we make it harder for child classes to migrate until the base class has migrated. But it is strictly better to permit the base class to migrate, because in projects you cannot be sure of all your subclasses out there in the wild, but you can be sure of all your base classes. -- ___ Python tracker <https://bugs.python.org/issue24412> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18054] Add more exception related assertions to unittest
Robert Collins added the comment: Hey, feel free to +nosy me on unittest things :). Julian pinged me on IRC about this - Jml and I would be delight to prepare a patchset for assertThat for unittest and as much of the core of Matchers as makes sense. The bare core can come in without any of the things that Michael is concerned about vis-a-vis unneeded complexity in testtools [e.g. nothing about arbitrary attachments is needed]. We can state from experience - both ours and users that have adopted it - that the decoupling assertThat brings is very effective at ending the forced-hierarchy-or-mixin mess that self-homed assertions brings. A while back we rewrote the entire core of testtools own assertions to be Matcher based :). https://testtools.readthedocs.org/en/latest/for-test-authors.html#matchers has the documentation on the Matcher protocol and what it looks like for users. Structurally, we decouple the raising of AssertionError from the act of determining whether a particular condition is met - this frees one from needing to have a reference to a test case to honour failureException, and from needing to subclass to share condition logic - unrelated test classes can how share condition logic by sharing a matcher definition. It also lends itself rather naturally to higher order definitions of matchers, as matchers are now standalone objects with no required connection to a test case : you can curry and compose matchers easily. We make matchers have a nice __str__, so that their structure can be printed out by assertThat when an error has occurred even if they have been curried or compose or defined much earlier. So - is there some interest in this? If so, I can put together a patchset with just the core, and let you see what it looks like. -- nosy: +rbcollins ___ Python tracker <http://bugs.python.org/issue18054> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18198] unittest discover should provide a way to define discovery at package level
Robert Collins added the comment: This is a duplicate of 16662 -- nosy: +rbcollins ___ Python tracker <http://bugs.python.org/issue18198> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18198] unittest discover should provide a way to define discovery at package level
Robert Collins added the comment: Well, spoke with vila on IRC. I content it's a duplicate because if load_tests in __init__ kicks in and supercedes discovery, I believe this bug would not be needed. -- ___ Python tracker <http://bugs.python.org/issue18198> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18232] running a suite with no tests is not an error
New submission from Robert Collins: In bug https://bugs.launchpad.net/subunit/+bug/586176 I recorded a user request - that if no tests are found, tools consuming subunit streams should be able to consider that an error. There is an analogous situation though, which is that if discover returns without error, running the resulting suite is worthless, as it has no tests. This is a bit of a sliding slope - what if discover finds one test when there should be 1000's ? Anyhow, filing this because there's been a few times when things have gone completely wrong that it would have helped CI systems detect that. (For instance, the tests package missing entirely, but tests were being scanned in the whole source tree, so no discover level error occurred). I'm thinking I'll add a '--min-tests=X' parameter to unittest.main, with the semantic that if there are less than X tests executed, the test run will be considered a failure, and folk can set this to 1 for the special case, or any arbitrary figure that they want for larger suites. -- messages: 191282 nosy: michael.foord, rbcollins priority: normal severity: normal status: open title: running a suite with no tests is not an error ___ Python tracker <http://bugs.python.org/issue18232> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18526] Add resource management/guarding to unittest
Changes by Robert Collins : -- nosy: +rbcollins ___ Python tracker <http://bugs.python.org/issue18526> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18526] Add resource management/guarding to unittest
Robert Collins added the comment: I'll do a full review shortly, but at the conceptual level, I don't see any benefit in making a new SkipTest base class, other than instantly breaking other runners when an unknown exception is raised. SkipTest is part of the API. Making a new exception part of the API requires a strong reason - not just 'we want to show it differently' but 'we want the runner to behave differently. -- ___ Python tracker <http://bugs.python.org/issue18526> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16058] ConfigParser no longer deepcopy compatible in 2.7
New submission from Robert Collins: In 2.6 deepcopy(ConfigParser) worked, in 2.7 it doesn't due to the _optcre variable which is a compiled regex pattern. -- components: Library (Lib) messages: 171364 nosy: rbcollins priority: normal severity: normal status: open title: ConfigParser no longer deepcopy compatible in 2.7 versions: Python 2.7 ___ Python tracker <http://bugs.python.org/issue16058> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16288] TextTestResult uses TestCase.__str__() which isn't customisable (vs id() or shortDescription())
New submission from Robert Collins: TextTestRunner calls str(TestCase) directly, which makes it hard for testscenarios to rename the test cases as it parameterises them (because __str__ is a descriptor). While testscenarios could use a decorator instead, thats undesirable as the test case object would still need to be customised so that calls to self.id() and self.shortDescription() inside it still return consistent information. So the relevant code is this: def getDescription(self, test): 41 if self.descriptions: 42 return test.shortDescription() or str(test) 43 else: 44 return str(test) What I'd like is to have this be something like: 41 if self.descriptions: 42 return test.shortDescription() or test.id() 43 else: 44 return test.id() Which would let testscenarios adjust both shortDescriptions and id, and Just Work. -- messages: 173352 nosy: michael.foord, rbcollins priority: normal severity: normal status: open title: TextTestResult uses TestCase.__str__() which isn't customisable (vs id() or shortDescription()) versions: Python 2.7, Python 3.1, Python 3.2, Python 3.3, Python 3.4, Python 3.5 ___ Python tracker <http://bugs.python.org/issue16288> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16288] TextTestResult uses TestCase.__str__() which isn't customisable (vs id() or shortDescription())
Robert Collins added the comment: Or anther way this could be done would be to make TestCase.__str__ call self.id(), and then __str__ never needs to be adjusted - id() or shortDescription are the only things to tweak. -- ___ Python tracker <http://bugs.python.org/issue16288> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16288] TextTestResult uses TestCase.__str__() which isn't customisable (vs id() or shortDescription())
Robert Collins added the comment: They aren't descriptors? They certainly aren't normal methods: >>> class foo(object): ... def __str__(self): return "foo" ... >>> f = foo() >>> f.__str__ = lambda: "bar" >>> str(f) 'foo' I *thought* the mechanism by which they can only be replaced by altering the class *was* descriptors, but I gladly seek better info! _testMethodName is overloaded though: its both 'what is the test method' and 'part of the description or id of the test', so resetting it would be - at best - risky when only changing the description or id. -- type: enhancement -> ___ Python tracker <http://bugs.python.org/issue16288> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16288] TextTestResult uses TestCase.__str__() which isn't customisable (vs id() or shortDescription())
Robert Collins added the comment: testscenarios copies the tests, it doesn't call the constructor for the class; this makes things a lot simpler than trying to reconstruct whatever state the object may have from scratch again. As for str(test) and test.id() being different - well sure they are today, but I don't know that the str(test) format is /useful/ today, as its not a particularly good str() anyhow. It doesn't identify that its a test instance even. This suggests two alternatives to me: - decide that we want three things: id, friendly-id and shortDescription, and have three non-magic methods, which TextTestRunner can call depending on what the user wants to see. - decide we want two things, id and shortDescription, and TextTestRunner can combine these things to meet the users request. (e.g. id + ' ' + shortDescription) And separately, as the __str__ isn't particularly good anyhow, perhaps we should take the opportunity to think about what we want from it and adjust it. -- type: enhancement -> ___ Python tracker <http://bugs.python.org/issue16288> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16288] TextTestResult uses TestCase.__str__() which isn't customisable (vs id() or shortDescription())
Robert Collins added the comment: @Michael I'll put a patch together next week then. @R.david.murray no idea - but I've refreshed the page, we'll if it behaves better. My guess would be a buggy in-flight-collision detection in the issue tracker code. -- ___ Python tracker <http://bugs.python.org/issue16288> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
New submission from Robert Collins: In loader.py: if fnmatch(path, pattern): # only check load_tests if the package directory itself matches the filter name = self._get_name_from_path(full_path) package = self._get_module_from_name(name) load_tests = getattr(package, 'load_tests', None) tests = self.loadTestsFromModule(package, use_load_tests=False) But pattern is test*.py by default, and packages will never match that. Its not at all clear why this is special cased at all, but if it is, we'll need a separate pattern. (Its not special cased in the bzrlib implementation that acted as the initial implementation). -- messages: 177327 nosy: rbcollins priority: normal severity: normal status: open title: load_tests not invoked in package/__init__.py ___ Python tracker <http://bugs.python.org/issue16662> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12600] Add example of using load_tests to parameterise Test Cases
Robert Collins added the comment: BTW I'm very happy with testscenarios (on pypi) for this, modulo one small issue which is the use of __str__ by the stdlib [which isn't easily overridable - there is a separate issue on that]. I'd be delighted to have testscenarios be in the stdlib, if thats desirable. -- nosy: +rbcollins ___ Python tracker <http://bugs.python.org/issue12600> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
Robert Collins added the comment: I have a package with tests in it. If the tests match test*.py, they are loaded, and load_tests within each one called. But load_tests on the package isn't called. If I change the pattern supplied by the user to match the package, then the tests within adjacent packages that don't have a load_tests hook but have files called test*.py will no longer match, but the package will match. My preference would be for the special case to just be removed, and load_tests called if it exists: its existence is enough of an opt-in. Failing that, having two distinct fn patterns, one for packages and one for filenames (note the difference: one operates in the python namespace, one the filesystem namespace), would suffice. -- ___ Python tracker <http://bugs.python.org/issue16662> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16709] unittest discover order is filesystem specific - hard to reproduce
New submission from Robert Collins: Openstack recently switched from nose to using discover. discover walks the filesystem using os.listdir(), and that is just a thin layer over readdir. On ext3/ext4 filesystems, readdir is in an arbitrary order dependent on file insertion into the directory if dir_index is enabled (which is the default). This means that files are loaded in an order that isn't reproducable by other developers, so bad tests that have isolation issues can be very tricky to track down. Just wrapping the os.listdir() in sorted() would be sufficient to make this robust and repeatable, and avoid the headache. -- components: Library (Lib) messages: 177675 nosy: ezio.melotti, michael.foord, pitrou, rbcollins priority: normal severity: normal status: open title: unittest discover order is filesystem specific - hard to reproduce versions: Python 2.7, Python 3.1, Python 3.2, Python 3.3, Python 3.4, Python 3.5 ___ Python tracker <http://bugs.python.org/issue16709> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19746] No introspective way to detect ModuleImportFailure
New submission from Robert Collins: https://bugs.launchpad.net/testtools/+bug/1245672 was filed on testtools recently. It would be easier to fix that if there was some way that something loading a test suite could check to see if there were import errors. The current code nicely works in the case where folk run the tests - so we should keep that behaviour, but also accumulate a list somewhere. One possibility would be on the returned top level suite; another place would be on the loader itself. Or a new return type where we return a tuple of (suite, failures), but thats a more intrusive API change. Any preference about how to solve this? I will work up a patch given some minor direction. -- components: Library (Lib) messages: 204172 nosy: michael.foord, rbcollins priority: normal severity: normal status: open title: No introspective way to detect ModuleImportFailure type: enhancement versions: Python 3.5 ___ Python tracker <http://bugs.python.org/issue19746> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
Robert Collins added the comment: I think we rather need a test that using a load_tests hook to recursively load and transform a subdir works. Hacking on that now. -- ___ Python tracker <http://bugs.python.org/issue16662> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
Robert Collins added the comment: Ok, implementation I'm happy with is up in https://bitbucket.org/rbtcollins/cpython/commits/bbf2eb26dda8f3538893bf3dc33154089f37f99d -- hgrepos: +269 Added file: http://bugs.python.org/file36482/16662_passing_tests.diff ___ Python tracker <http://bugs.python.org/issue16662> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
Robert Collins added the comment: The doc part of the patch was assuming this would be in 3.4 which it wasn't. Updated to 3.5. Also found a corner case - when packages were imported the _get_module_from_name method was not guarded for un-importable modules. This is strictly a separate issue, but since we'll now import considerably more modules, it seemed prudent to fix it at the same time. -- Added file: http://bugs.python.org/file36491/16662_passing_tests_full.diff ___ Python tracker <http://bugs.python.org/issue16662> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
Robert Collins added the comment:
Thanks for landing this barry, there's a couple quirks with your improvements -
loadTestsFromModule(mod, foo, bar) will raise a TypeError but not warn about
foo the way loadTestsFromModule(mod, foo) will.
Secondly, the TypeError has an off-by-one error in its output:
loadTestsFromModule(mod, foo, bar) will claim 2 arguments were passed. Three
were.
diff -r d0ff527c53da Lib/unittest/loader.py
--- a/Lib/unittest/loader.pyMon Sep 08 14:21:37 2014 -0400
+++ b/Lib/unittest/loader.pyTue Sep 09 07:32:05 2014 +1200
@@ -79,12 +79,12 @@
# use_load_tests argument. For backward compatibility, we still
# accept the argument (which can also be the first position) but we
# ignore it and issue a deprecation warning if it's present.
-if len(args) == 1 or 'use_load_tests' in kws:
+if len(args) or 'use_load_tests' in kws:
warnings.warn('use_load_tests is deprecated and ignored',
DeprecationWarning)
kws.pop('use_load_tests', None)
if len(args) > 1:
-raise TypeError('loadTestsFromModule() takes 1 positional argument
but {} were given'.format(len(args)))
+raise TypeError('loadTestsFromModule() takes 1 positional argument
but {} were given'.format(len(args) + 1))
if len(kws) != 0:
# Since the keyword arguments are unsorted (see PEP 468), just
# pick the alphabetically sorted first argument to complain about,
diff -r d0ff527c53da Lib/unittest/test/test_loader.py
--- a/Lib/unittest/test/test_loader.py Mon Sep 08 14:21:37 2014 -0400
+++ b/Lib/unittest/test/test_loader.py Tue Sep 09 07:32:05 2014 +1200
@@ -272,7 +272,7 @@
# however use_load_tests (which sorts first) is ignored.
self.assertEqual(
str(cm.exception),
-'loadTestsFromModule() takes 1 positional argument but 2 were
given')
+'loadTestsFromModule() takes 1 positional argument but 3 were
given')
@warningregistry
def test_loadTestsFromModule__use_load_tests_other_bad_keyword(self):
--
___
Python tracker
<http://bugs.python.org/issue16662>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
Robert Collins added the comment: OH! One more thing I just spotted, which is that this change causes non-'discover' unittest test loading to invoke load_tests. IMO this is the Right Thing - its what I intended when I described the protocol a few years back, but we should document it, no? -- ___ Python tracker <http://bugs.python.org/issue16662> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17401] io.FileIO closefd parameter is not documented nor shown in repr
Robert Collins added the comment: Its more than just a docs issue - "AFAICT it isn't possible to tell if closefd is set after the object is created." The presence of the parameter in the signature is there, but it isn't documented *where the bulk of the FileIO parameters are* - there are docs for mode for instance. Why would we force users to follow breadcrumbs for one parameter but not other ones? -- ___ Python tracker <http://bugs.python.org/issue17401> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17401] io.FileIO closefd parameter is not documented nor shown in repr
Robert Collins added the comment: Oh - the the 'open' function docs are fine - they are just a pointer. I was specifically referring to the class docs around line 513 of Doc/library/io.rst. Attached is a patch that changes repr to show this attribute and extends the docs to document this as part of FileIO. -- keywords: +patch Added file: http://bugs.python.org/file36574/issue17401.patch ___ Python tracker <http://bugs.python.org/issue17401> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
Robert Collins added the comment: @michael - ah I think I inverted the sense of the old parameter. It was defaulting True. So - no need to document anything extra:) -- ___ Python tracker <http://bugs.python.org/issue16662> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19746] No introspective way to detect ModuleImportFailure in unittest
Robert Collins added the comment: Here is an implementation. I'm probably missing some finesse in the docs. -- keywords: +patch Added file: http://bugs.python.org/file36577/issue19746.patch ___ Python tracker <http://bugs.python.org/issue19746> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7559] TestLoader.loadTestsFromName swallows import errors
Robert Collins added the comment: I've just put a patch up for the related issue http://bugs.python.org/issue19746 I'll poke at this one briefly now, since I'm across the related code. -- ___ Python tracker <http://bugs.python.org/issue7559> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7559] TestLoader.loadTestsFromName swallows import errors
Robert Collins added the comment: Ok, here is an implementation that I believe covers everything Michael wanted. I examined the other patches, and can rearrange my implementation to be more like them if desired - but at the heart of this this bug really has two requested changes: - deferred reporting of error per Michaels request - report missing attributes on packages as an ImportError (if one occurred) and thus my implementation focuses on those changes. -- Added file: http://bugs.python.org/file36578/issue7559.patch ___ Python tracker <http://bugs.python.org/issue7559> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18232] running a suite with no tests is not an error
Robert Collins added the comment: @Terry in principle you're right, there are an arbitrary number of things that can go wrong, but in practice what we see is either catastrophic failure where nothing is loaded at all *and* no error is returned or localised failure where the deferred reporting of failed imports serves quite well enough. The former is caused by things like the wrong path in a configuration file. @ezio sure - a boolean option would meet the needs reported to me, I was suggesting a specific implementation in an attempt to be generic enough to not need to maintain two things if more was added in future. -- ___ Python tracker <http://bugs.python.org/issue18232> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7559] TestLoader.loadTestsFromName swallows import errors
Robert Collins added the comment: You may need to apply the patch from http://bugs.python.org/issue19746 first as well - I was testing with both applied. -- ___ Python tracker <http://bugs.python.org/issue7559> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7559] TestLoader.loadTestsFromName swallows import errors
Robert Collins added the comment: This is what I see in my tree: E == ERROR: test_os (unittest.loader.ModuleImportFailure) -- Traceback (most recent call last): File "/home/robertc/work/cpython/Lib/unittest/case.py", line 58, in testPartExecutor yield File "/home/robertc/work/cpython/Lib/unittest/case.py", line 577, in run testMethod() File "/home/robertc/work/cpython/Lib/unittest/loader.py", line 36, in testFailure raise exception ImportError: Failed to import test module: test_os Traceback (most recent call last): File "/home/robertc/work/cpython/Lib/unittest/loader.py", line 146, in loadTestsFromName module = __import__(module_name) File "/home/robertc/work/cpython/Lib/test/test_os.py", line 5, in import broken ImportError: No module named 'broken' -- ___ Python tracker <http://bugs.python.org/issue7559> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7559] TestLoader.loadTestsFromName swallows import errors
Robert Collins added the comment: Raced with your comment. Great - and thanks! -- ___ Python tracker <http://bugs.python.org/issue7559> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19746] No introspective way to detect ModuleImportFailure in unittest
Robert Collins added the comment:
Thanks; I'm still learning how to get the system here to jump appropriately :).
I thought I'd told hg to reset me to trunk...
"You are right about the docs. Reading that, I thought it was saying that
errors would have a list of the errors that show up in the summary as Errors=N,
and not the ones that show up as Failures=N, which of course is completely off
base for two reasons (but, then, I can never remember the difference between
Failure and Error and always ignore what type the test failures are)."
Ah, so this is specifically about *loader* errors, nothing to do with the ERROR
and FAILURE concepts for the TestResult class; that definitely needs to be made
more clear.
"Anyway, you probably want to talk about actual error types. I understand
ImportError, but I have no idea what would trigger the 'Failed to call
load_tests' error. Nor is it clear what would be a fatal error (easier just to
say which errors are trapped, I think)."
'Failed to call load_tests' is an existing error that can be triggered if a
load_tests method errors.
e.g. put this in a test_foo.py:
def load_tests(loader, tests, pattern):
raise Exception('fred')
to see it with/without the patch. I'll take a stab at improving the docs in a
bit.
"It should also be mentioned that the contents of the list are an error message
followed by a full traceback. And, frankly, I'm not sure why this is useful,
and in particular why this implementation satisfies your use case."
Ah! So I have an external runner which can enumerate the tests without running
them. This is useful when the test suite is being distributed across many
machines (simple hash based distribution can have very uneven running times, so
enumerating the tests that need to run then scheduling based on runtime (or
other metadata) gets better results). If the test suite can't be imported I
need to show the failure of the import to users so they can fix it, but since
the test suite may take hours (or even not be runnable locally) I need to do
this without running the tests. Thus a simple list of the tracebacks
encountered loading the test suite is sufficient. Where would be a good place
to make this clearer?
--
___
Python tracker
<http://bugs.python.org/issue19746>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22457] load_tests not invoked in root __init__.py when start=package root
New submission from Robert Collins:
python -m unittest discover -t . foo
where foo is a package
will not trigger load_tests in foo/__init__.py.
To reproduce:
mkdir -p demo/tests
cd demo
cat < tests/__init__.py
import sys
import os
def load_tests(loader, tests, pattern):
print("HI WE ARE LOADING!")
this_dir = os.path.dirname(__file__)
tests.addTest(loader.discover(start_dir=this_dir, pattern=pattern))
return tests
EOF
python -m unittest discover -t . tests
--
messages: 227250
nosy: rbcollins
priority: normal
severity: normal
status: open
title: load_tests not invoked in root __init__.py when start=package root
___
Python tracker
<http://bugs.python.org/issue22457>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19746] No introspective way to detect ModuleImportFailure in unittest
Robert Collins added the comment: I can certainly write the reporter glue to work with either a string or a full reference. Note that the existing late-reporting glue captures the import error into a string, and then raises an exception containing that string - so what I've done is consistent with that. As for why the original code is like that - well I've had plenty of bad experiences with memory loops due to objects held in stack frames of exceptions, I don't like to keep long lived references to them, and I wager Michael has had the same experience. -- ___ Python tracker <http://bugs.python.org/issue19746> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22457] load_tests not invoked in root __init__.py when start=package root
Robert Collins added the comment: This should fix this issue :) -- keywords: +patch Added file: http://bugs.python.org/file36694/issue22457.patch ___ Python tracker <http://bugs.python.org/issue22457> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
Robert Collins added the comment: I've managed to get a windows setup working. Its my mini-vfs which needs to be Windows aware (because the abs path of /foo is C:\\foo). I'll work up a patch tomorrowish. -- ___ Python tracker <http://bugs.python.org/issue16662> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
Robert Collins added the comment: Fix up the tests patch - tested on windows 7. -- Added file: http://bugs.python.org/file36713/fix-windows-tests.patch ___ Python tracker <http://bugs.python.org/issue16662> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
Robert Collins added the comment: bah, wrong extension to trigger review code :) -- Added file: http://bugs.python.org/file36714/fix-windows-tests.diff ___ Python tracker <http://bugs.python.org/issue16662> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16662] load_tests not invoked in package/__init__.py
Changes by Robert Collins : Removed file: http://bugs.python.org/file36713/fix-windows-tests.patch ___ Python tracker <http://bugs.python.org/issue16662> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22489] .gitignore file
New submission from Robert Collins: The .gitignore file was missing some build products on windows. The attached patch makes the tree be clean after doing a debug build. -- files: windows-git-ignore.diff keywords: patch messages: 227498 nosy: rbcollins priority: normal severity: normal status: open title: .gitignore file Added file: http://bugs.python.org/file36715/windows-git-ignore.diff ___ Python tracker <http://bugs.python.org/issue22489> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22457] load_tests not invoked in root __init__.py when start=package root
Robert Collins added the comment: Updated patch - fixes windows tests for this patch. -- Added file: http://bugs.python.org/file36716/issue22457.diff ___ Python tracker <http://bugs.python.org/issue22457> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19746] No introspective way to detect ModuleImportFailure in unittest
Robert Collins added the comment: Right: the existing code stringifies the original exception and creates an exception object and a closure def test_thing(self): raise exception_obj but that has the stringified original exception. -- ___ Python tracker <http://bugs.python.org/issue19746> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22264] Add wsgiref.util.dump_wsgistr & load_wsgistr
Robert Collins added the comment:
So this looks like its going to instantly create bugs in programs that use it.
HTTP/1.1 headers are one of:
latin1
MIME encoded (RFC2047)
invalid and working only by accident
HTTP/2 doesn't change this.
An API that encourages folk to encode into utf8 and then put that in their
headers is problematic.
Consider:
def dump_wsgistr(data, encoding, errors='strict'):
data.encode(encoding, errors).decode('iso-8859-1')
This takes a string that one wants to put into a header value, encodes it with
a *user specified encoding*, then decodes that into iso-8859-1 [at which point
it can be encoded back to octets by the wsgi server before putting on the wire].
But this is fundamentally wrong in the common case: either 'data' is itself
suitable as a header value (e.g. it is ASCII - recommended per RFC7230 section
3.2.4) or 'data' needs encoding via RFC 2047 encoding not via utf8.
There are a few special cases where folk have incorrectly shoved utf8 into
header values and we need to make it possible for folk working within WSGI to
do that - which is why the API is the way it is - but we shouldn't make it
*easier* for them to do the wrong thing.
I'd support an API that DTRT here by taking a string, tries US_ASCII, with
fallback to MIME encoded with utf8 as the encoding parameter.
--
nosy: +rbcollins
___
Python tracker
<http://bugs.python.org/issue22264>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21472] Fix wsgiref handling of absolute HTTP Request-URI
Robert Collins added the comment: FWIW we probably need to capture the original unaltered URL somewhere, but also ensure that PATH_INFO is always a relative path. One should be able to implement a proxy in WSGI (because thats just another specialised app), and doing that today requires special handling depending on the WSGI container, which isn't great for consistency. On security; Host header <-> url host mismatches occur when the host to which a request is sent != the url; this is expected only in the case of forward proxies: any other time it would indeed be a smuggling attack, trying to find mismatches between acls and access in servers - this is another reason to consolidate things so that wsgi apps can rely on urls looking consistent. -- nosy: +rbcollins ___ Python tracker <http://bugs.python.org/issue21472> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21472] Fix wsgiref handling of absolute HTTP Request-URI
Robert Collins added the comment: Oh, also - while its tempting to say that it doesn't matter whether we take the urls host portion, or the host header or the server name - it does. Deployments that look like: LB/Firewall <-> backend container -> WSGI app are likely to have assumptions within the WSGI app about what sort of urls they expect to reach them. We can mitigate this by being very explicit about whatever solution we have. -- ___ Python tracker <http://bugs.python.org/issue21472> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22264] Add wsgiref.util.dump_wsgistr & load_wsgistr
Robert Collins added the comment: So I guess the API concern I have is that there are two cases: - common spec compliant - US-ASCII + RFC2047 - dealing with exceptions - UTF8 or otherwise The former totally makes sense as a codec, though the current email implementation of it isn't quite a codec. The latter almost wants to be a separate API, which I may propose as part of WSGI for HTTP/2; nevertheless in PEP- its integral to the main API because of the bytes-encoded-as-codepoints-00-ff solution. So, perhaps: - a codec or something looking like it that covers the common case this would not permit specifying codecs on decode, for instance [since RFC2047 is self describing]. - documentation for the uncommon case. *Possibly* a helper function. -- ___ Python tracker <http://bugs.python.org/issue22264> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20687] Change in expectedFailure breaks testtools
Robert Collins added the comment: I concur that this is a regression - " unittest.expectedFailure() Mark the test as an expected failure. If the test fails when run, the test is not counted as a failure. " is in the public docs for the unittest module, and depending on a private attribute to be set is a bit of an abstraction violation - at the least it should handle it not being set (for any older test unittest compatible testcase object). -- ___ Python tracker <http://bugs.python.org/issue20687> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20687] Change in expectedFailure breaks testtools
Robert Collins added the comment: Oh! I didn't realise it was due to us looking at a private exception - I haven't been given a traceback to review, just the statement of a problem We shouldn't have done that(and *Definitely* should have filed a bug that we needed to). So - I think the question should become 'can we cope with this in testtools' - we'll issue a point release to ensure compat, assuming it's at all possible. -- ___ Python tracker <http://bugs.python.org/issue20687> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7205] bz2file deadlock
New submission from Robert Collins : There is a systemic bug in BZ2File where the GIL is released to perform compression work, and any other thread calling into BZ2File will deadlock. We noticed in the write method, but inspection of the code makes it clear that its systemic. The problem is pretty simple. Say you have two threads and one bz2file object. One calls write(), the other calls (say) seek(), but it could be write() or other methods too. Now, its pretty clear that the question 'should these two threads get serialised' could be contentious. So lets put that aside by saying 'raising an exception or serialising in arbitrary order would be ok'. What happens today is: t1:bz2file.write bz2file.lock.acquire gil-release bz2compression starts t2:gil-acquired bz2file.seek bz2file.lock.acquire(wait=1) <- this thread is stuck now, and has the GIL t1:bz2compression finishes gil.acquire <- this thread is stuck now, waiting for the GIL If any owner of the bz2file object lock will release the GIL, *all* routines that attempt to lock the bz2file object have to release the GIL if they can't get the lock - blocking won't work. I'm not familiar enough with the python threading API to know whether its safe to call without the GIL. If its not then clearly it can't be used to work with getting the GIL, and lower layer locks should be used. -- components: Extension Modules files: bz2fail.py messages: 94462 nosy: barry, rbcollins, statik severity: normal status: open title: bz2file deadlock type: crash Added file: http://bugs.python.org/file15196/bz2fail.py ___ Python tracker <http://bugs.python.org/issue7205> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7205] bz2file deadlock
Robert Collins added the comment: On Sun, 2009-10-25 at 22:00 +, Antoine Pitrou wrote: > Antoine Pitrou added the comment: > > Thanks, nice catch. Yeah :). > versions: +Python 2.6, Python 2.7, Python 3.1, Python 3.2 Python 2.5 is also affected - its what we're running on the server that broke :) -Rob -- ___ Python tracker <http://bugs.python.org/issue7205> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7205] bz2file deadlock
Robert Collins added the comment: On Sun, 2009-10-25 at 22:27 +, Antoine Pitrou wrote: > Antoine Pitrou added the comment: > > > Python 2.5 is also affected - its what we're running on the server that > > broke :) > > Yes, but it doesn't receive any bug fixes anymore -- only security > fixes. Ok, we'll work around the issue there then ;) -Rob -- ___ Python tracker <http://bugs.python.org/issue7205> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7205] bz2file deadlock
Robert Collins added the comment: On Mon, 2009-10-26 at 19:23 +, Antoine Pitrou wrote: > Antoine Pitrou added the comment: > > Here is a patch. Looks fine to me assuming that the locking functions can be used outside the GIL. On the test side, the case I supplied was low noise for me - I'd hesitate to do as much compression as you are (50 times more) unless you saw it spuriously pass a lot - the nature of the deadlock isn't dependent on races so much as simple scheduling - as long as the seocnd thread is scheduled before the first thread completes compressing the deadlock will occur. -Rob -- ___ Python tracker <http://bugs.python.org/issue7205> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7205] bz2file deadlock
Robert Collins added the comment: On Mon, 2009-10-26 at 21:27 +, Antoine Pitrou wrote: > Antoine Pitrou added the comment: > Well, your test case often succeeded here, so I decided on a more > aggressive variation. fair enough, if its needed - its needed :) -Rob -- ___ Python tracker <http://bugs.python.org/issue7205> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1515] deepcopy doesn't copy instance methods
Robert Collins added the comment: Ran into this trying to do some test isolation stuff. Notwithstanding the questions about 'why', this is a clear limitation hat can be solved quite simply - is there any harm that will occur if we fix it? I've attached a patch, with a test (in the style of the current tests) which shows the copy works correctly. -- keywords: +patch nosy: +rbcollins Added file: http://bugs.python.org/file15405/issue1515.patch ___ Python tracker <http://bugs.python.org/issue1515> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1515] deepcopy doesn't copy instance methods
Robert Collins added the comment: This affects 2.7 too. -- versions: +Python 2.7 ___ Python tracker <http://bugs.python.org/issue1515> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1515] deepcopy doesn't copy instance methods
Robert Collins added the comment: @Antoine, I agree that the tests for copy should be a proper unit test; that seems orthogonal to this patch though :) I don't have a checkout of 3 at the moment, but do you think the test failure on 3 is shallow or deep? -- ___ Python tracker <http://bugs.python.org/issue1515> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1515] deepcopy doesn't copy instance methods
Robert Collins added the comment: Oh man, I looked for a regular unit test - sorry that I missed it. Bah. I've added a call to the method and moved it into test_copy. -- Added file: http://bugs.python.org/file15406/issue1515.patch ___ Python tracker <http://bugs.python.org/issue1515> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7409] cleanup now deepcopy copies instance methods
New submission from Robert Collins : With deepcopy fixed, I ran across this little fixable component. -- components: Library (Lib) files: deepcopy-works.patch keywords: patch messages: 95823 nosy: rbcollins severity: normal status: open title: cleanup now deepcopy copies instance methods versions: Python 2.7, Python 3.2 Added file: http://bugs.python.org/file15417/deepcopy-works.patch ___ Python tracker <http://bugs.python.org/issue7409> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7410] deepcopy of itertools.count resets the count
New submission from Robert Collins : >>> from copy import deepcopy >>> from itertools import count >>> c = count() >>> c.next() 0 >>> deepcopy(c) count(0) >>> c.next() 1 >>> deepcopy(c) count(0) >>> c count(2) >>> deepcopy(c).next() 0 I don't see any reason why these shouldn't be deepcopyable (or picklable for that reason - and that fails too) -- components: Library (Lib) messages: 95830 nosy: rbcollins severity: normal status: open title: deepcopy of itertools.count resets the count versions: Python 2.7, Python 3.2 ___ Python tracker <http://bugs.python.org/issue7410> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2422] Automatically disable pymalloc when running under valgrind
Changes by Robert Collins : -- nosy: +rbcollins ___ Python tracker <http://bugs.python.org/issue2422> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6108] unicode(exception) and str(exception) should return the same message on Py2.6
Robert Collins added the comment:
"2) 0 args, e = MyException(), with overridden __str__:
py2.5 : str(e) -> 'ascii' or error; unicode(e) -> u'ascii' or error;
py2.6 : str(e) -> 'ascii' or error; unicode(e) -> u''
desired: str(e) -> 'ascii' or error; unicode(e) -> u'ascii' or error;
Note: py2.5 behaviour is better: if __str__ returns an ascii string
(including ''), unicode(e) should return the same string decoded, if
__str__ returns a non-ascii string, both should raise an error.
"
I'm not sure how you justify raising an unnecessary error when trying to
stringify an exception as being 'better'.
__str__ should not decode its arguments if they are already strings:
they may be valid data for the user even if they are not decodable (and
note that an implicit decode may try to decode('ascii') which is totally
useless.
__str__ and __unicode__ are /different/ things, claiming they have to
behave the same is equivalent to claiming either that we don't need
unicode, or that we don't need binary data.
Surely there is space for both things, which does imply that
unicode(str(e)) != unicode(e).
Why _should_ that be the same anyway?
--
nosy: +rbcollins
___
Python tracker
<http://bugs.python.org/issue6108>
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7501] python -m unittest path_to_suite_function errors
New submission from Robert Collins : :!python -m unittest foo.test_suite Traceback (most recent call last): File "/usr/lib/python2.6/runpy.py", line 122, in _run_module_as_main "__main__", fname, loader, pkg_name) File "/usr/lib/python2.6/runpy.py", line 34, in _run_code exec code in run_globals File "/usr/lib/python2.6/unittest.py", line 875, in main(module=None) File "/usr/lib/python2.6/unittest.py", line 816, in __init__ self.parseArgs(argv) File "/usr/lib/python2.6/unittest.py", line 843, in parseArgs self.createTests() File "/usr/lib/python2.6/unittest.py", line 849, in createTests self.module) File "/usr/lib/python2.6/unittest.py", line 613, in loadTestsFromNames suites = [self.loadTestsFromName(name, module) for name in names] File "/usr/lib/python2.6/unittest.py", line 605, in loadTestsFromName (obj, test)) TypeError: calling returned , ]>, not a test where foo.py: def test_suite(): loader = unittest.TestLoader() tests = loader.loadTestsFromName(__name__) result = loader.suiteClass() result.addTests(generate_scenarios(tests)) return result -- components: Library (Lib) messages: 96364 nosy: rbcollins severity: normal status: open title: python -m unittest path_to_suite_function errors type: crash versions: Python 2.6, Python 2.7, Python 3.0, Python 3.1, Python 3.2 ___ Python tracker <http://bugs.python.org/issue7501> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7559] TestLoader.loadTestsFromName swallows import errors
New submission from Robert Collins : Say I have a test module test_foo, which fails to import with ImportError. A reason for this might be a misspelt import in that module. TestLoader().loadTestsFromName swallows the import error and instead crashes with: File "/usr/lib/python2.6/unittest.py", line 584, in loadTestsFromName parent, obj = obj, getattr(obj, part) AttributeError: 'module' object has no attribute 'test_foo' A better thing to do would be to keep the import error and if the next probe is an Attribute error, reraise the import error. An alternative would be to return a test which would then reraise the import error permitting the test suite to be constructed and execute but still reporting the error. -- components: Library (Lib) messages: 96770 nosy: rbcollins severity: normal status: open title: TestLoader.loadTestsFromName swallows import errors ___ Python tracker <http://bugs.python.org/issue7559> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7559] TestLoader.loadTestsFromName swallows import errors
Robert Collins added the comment: mkdir thing touch thing/__init__.py echo "import blert" > thing/test_foo.py python -m unittest thing.test_fooTraceback (most recent call last): File "/usr/lib/python2.6/runpy.py", line 122, in _run_module_as_main "__main__", fname, loader, pkg_name) File "/usr/lib/python2.6/runpy.py", line 34, in _run_code exec code in run_globals File "/usr/lib/python2.6/unittest.py", line 875, in main(module=None) File "/usr/lib/python2.6/unittest.py", line 816, in __init__ self.parseArgs(argv) File "/usr/lib/python2.6/unittest.py", line 843, in parseArgs self.createTests() File "/usr/lib/python2.6/unittest.py", line 849, in createTests self.module) File "/usr/lib/python2.6/unittest.py", line 613, in loadTestsFromNames suites = [self.loadTestsFromName(name, module) for name in names] File "/usr/lib/python2.6/unittest.py", line 584, in loadTestsFromName parent, obj = obj, getattr(obj, part) AttributeError: 'module' object has no attribute 'test_foo' -- ___ Python tracker <http://bugs.python.org/issue7559> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7559] TestLoader.loadTestsFromName swallows import errors
Robert Collins added the comment: I'm scratching an itch at the moment, I just noted this in passing ;) I'm partial to the 'turn it into a fake test case' approach, its what I would do if I get to it first. -- ___ Python tracker <http://bugs.python.org/issue7559> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins added the comment: It should run after tearDown so that teardown can do actions that may require cleanup; because the cleanups run in LIFO you can acquire resources in setUp and have cleanups clean them up, -- nosy: +rbcollins ___ Python tracker <http://bugs.python.org/issue5679> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins added the comment: Actually let me phrase that differently. standard practice for setUp is super.setUp() my_setup_code() and tearDown is my_teardown_code() super.tearDown() because of the LIFO need. If you imagine that clean ups are being done in the base class teardown, at the end of that method, then it becomes a lot more obvious why it should run after tearDown - because thats the right place, and because many users may be failing to call the base class tearDown which has historically done nothing. -- ___ Python tracker <http://bugs.python.org/issue5679> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5693] TestSuite.__iter__ is not hookable.
New submission from Robert Collins : Currently if you alter the way TestSuite iterates one must always implement countTestCases, run, debug etc. The attached patch would make this simpler. If this looks ok I'll write up a test for it. -- components: Library (Lib) files: testsuite.patch keywords: patch messages: 85435 nosy: rbcollins severity: normal status: open title: TestSuite.__iter__ is not hookable. type: feature request Added file: http://bugs.python.org/file13615/testsuite.patch ___ Python tracker <http://bugs.python.org/issue5693> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins added the comment: On Sat, 2009-04-04 at 22:09 +, Antoine Pitrou wrote: > Antoine Pitrou added the comment: > > teardown > > Why should they? It's only an implementation choice, and not a wise one > I would say (precisely because people are used to the fact that the > standard tearDown() method does nothing, and doesn't need to be called). > > I explained my proposal in terms of actual use cases, but I don't see > any actual use case of addCleanup() in your argument. I was arguing by analogy: if we were to implement addCleanup as something called at the end of the base class tearDown, then it would clearly support LIFO tearing down of anything people have done [except that we can't rely on them having called the base tearDown]. The next best thing then is to call it from run() after calling tearDown. If you want a worked example: --- class TestSample(TestCase): def setUp(self): dirname = mkdtemp() self.addCleanup(shutils.rmtree, dirname, ignore_errors=True) db = make_db(dirname) self.addCleanup(db.tearDown) --- This depends on db being torn down before the rmtree, or else the db teardown will blow up (and it must be torn down to release locks correctly on windows). -Rob -- ___ Python tracker <http://bugs.python.org/issue5679> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins added the comment: On Sat, 2009-04-04 at 23:06 +, Antoine Pitrou wrote: > Antoine Pitrou added the comment: > > > The main use case for addCleanup is resource allocation in tests. Why > > does this require clean ups to be executed before tearDown? > > If your cleanup relies on something which has been set up during setUp > and will be dropped during tearDown (a database connection, a temp dir, > an ssh session, whatever), then the cleanup must be run before the > teardown. I don't think you can sensibly define an ordering between setup/teardown and cleanups that works for all cases. Either cleanups happen after teardown, and then you can use them when allocating things in setup/run/teardown, or they happen before teardown and you cannot safely use them except in run because things setup by a child will not have been torn down when cleanups run. [see the two where-it-breaks sequences below]. The issue is that cleanups are not connected to the inheritance heirarchy. They are safe and trivial to use with resources that are not connected to each other *however* they are defined, but the interaction with things being freed in tearDown cannot meet all cases unless you have per-class-in the-MRO-queues (which is overly complex). I assert that its better to be able to use cleanups in all three test methods rather than only in run, all other things being equal. Our experience in bzr (we use this heavily, and migrated to it incrementally across our 17K fixture suite) is that we rarely need to use cleanups on dependent resources, and when we need to it has been very easy to migrate the dependent resource to use cleanups as well. -Rob sequence 1: cleanup after teardowns prevents using cleanups on things freed in teardown: setup self.foo = thing() run self.bar = something(self.foo) self.addCleanUp(free, self.bar) teardown unthing(self.foo) cleanups free(self.bar) *boom* self.foo is already gone sequence 2: cleanup before teardown prevents using cleanups in base class setup methods base.setup self.foo = thing() self.addCleanup(unthing, self.foo) setup super.setup() self.bar = something(self.foo) run assertWhatever cleanups unthing(self.foo) teardown free(self.bar) *boom* self.foo is already gone -- ___ Python tracker <http://bugs.python.org/issue5679> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins added the comment: On Sun, 2009-04-05 at 07:25 +, Garrett Cooper wrote: > Garrett Cooper added the comment: > > I think some perspective is required on this enhancement request. I > originally filed this issue -- http://bugs.python.org/issue5538 -- > because of the unneeded complexity involved with duplicating > teardown-related code in setUp because of a step in setUp failing. Indeed, and in bzr and other unittest extension libraries one of the first things done is to make tearDown be called always. > >From my perspective, there are two issues: > > - setUp failing doesn't cleanup on failure unless the test writer > explicitly adds cleanup logic. the proposed patch in 5679 runs cleanups always, so this shouldn't be an issue with the addCleanup functionality. > - cleanup shouldn't partially replace tearDown -- either supplement it > or completely replace it longterm. Otherwise the unittest code and > expectations associated with it will potentially confuse end users. I'm conceptually fine with this, certainly I've not written a tearDown for tests in bzr in a very long time. > Another thought: Why not have an option for defining a method called > `incrementalTearDown', which replaces `tearDown' from a functional > standpoint? A method like that would clearly convey that this is > designed to replace tearDown, it's not the same functionally, and would > ease migration over the long-term if people chose to use this design > when writing testcases. Its much more complex than addCleanup [it will be tied to the MRO], not as flexible (and in big suites that starts to matter a lot), and unable to trivially hook into actually used resources [which addCleanup does]. -Rob -- ___ Python tracker <http://bugs.python.org/issue5679> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins added the comment: On Sun, 2009-04-05 at 10:15 +, Antoine Pitrou wrote: > Antoine Pitrou added the comment: > > > Our experience in bzr (we use this heavily, and migrated to it > > incrementally across our 17K fixture suite) is that we rarely need to > > use cleanups on dependent resources, and when we need to it has been > > very easy to migrate the dependent resource to use cleanups as well. > > I'm baffled. If you say you don't care about the order, why are you > arguing at all? I didn't say I don't care; I do - I care that it is robust and hard to misuse. Having addCleanup() when called from a tearDown lead to cleanups not being called would be an easy route to misuse. > [...] > > sequence 2: cleanup before teardown prevents using cleanups in base > > class setup methods > > The point is that sequence 2 can already be emulated using careful > "try...finally" in tearDown, while sequence 1 cannot. That is, sequence > 1 *needs* the addCleanup, while for sequence 2 it is a mere additional > convenience. I don't understand; neither sequence works - they are showing how any choice [that retains the current simple proposed mechanism] cannot interact without some failure modes with tearDown. Whichever point we choose to have cleanups execute can be entirely emulated using careful try:finally: in tearDown methods, so surely this is not an argument for either order. -Rob -- ___ Python tracker <http://bugs.python.org/issue5679> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins added the comment: On Sun, 2009-04-05 at 21:31 +, Antoine Pitrou wrote: > Antoine Pitrou added the comment: > > > So, we are talking about adding a feature that could cause problem whether > > cleanup is performed before tearDown or after tearDown. Don't we risk > > confusing developers who are not familiar with the cleanup order? > > Well, we could do both. Call cleanups before tearDown (all the while > popping them), call tearDown, and call the new cleanups. I think this makes cleanups harder to explain, but yes we could do it. (Its harder because its no longer clearly a LIFO as cleanups added before teardown at not executed last). > If the cleanup machinery is written carefully enough, you may even be > able to add another cleanup during a cleanup :-) I've been meaning to tweak the patch - rather than +for function, args, kwargs in reversed(self._cleanups): it should be +while self._cleanups: +function, args, kwargs = self._cleanups.pop(-1) precisely to allow this. The point I've been trying to make isn't just that after everything is clearly best (which is subjective), but that dropping cleanups from user code is clearly wrong (something I hope we all agree on) and that its harder to have bugs like that if cleanups run after teardown. @Antoine I appreciate that you feel very strongly about this; I'm at a bit of a loss why, given that I've been working with this feature for over 3 *years* and we've not once had headaches or confusion related to the discussed sequencing failure modes - and our code runs it after all the user code. I've tried my level best to document why it works best after running after all the user code, but ultimately whoever is committing this code to python needs to make a call about what you think is best. It would be nice if you can learn from our experience, but I'm about out of energy to discuss this; certainly when we're down to authority by posturing it seems like fruitful discussion is rather over. @Michael - you may have a convenience function that uses addCleanup - if that gets called during tearDown then while its a little odd its still correct for cleanups the convenience function added to get called. It's only in the light of having to really explain why it should be afterwards that I've realised we had a latent bug - fixed in the change above [and a test for it is pretty obvious]. -Rob -- ___ Python tracker <http://bugs.python.org/issue5679> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
