This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2013年11月24日 03:16 by rbcollins, last changed 2022年04月11日 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| issue19746.patch | rbcollins, 2014年09月08日 21:27 | |||
| issue19746-rebased.patch | r.david.murray, 2014年09月10日 01:11 | review | ||
| Messages (11) | |||
|---|---|---|---|
| msg204172 - (view) | Author: Robert Collins (rbcollins) * (Python committer) | Date: 2013年11月24日 03:16 | |
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. |
|||
| msg204265 - (view) | Author: Michael Foord (michael.foord) * (Python committer) | Date: 2013年11月24日 20:33 | |
Seems like a perfectly reasonable request. I've no particular preference on an api for this though. |
|||
| msg226612 - (view) | Author: Robert Collins (rbcollins) * (Python committer) | Date: 2014年09月08日 21:27 | |
Here is an implementation. I'm probably missing some finesse in the docs. |
|||
| msg226671 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2014年09月10日 01:36 | |
Your patch isn't diffed against a revision from the cpython repo, and apparently didn't apply cleanly to tip, so no review link was generated. I uploaded a rebased patch to review, but don't actually have any line by line comments. 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). 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). 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. (I'm more interested in the issue 7559 patch, but since it based on this one I don't think I can just rebase it to review it.) |
|||
| msg226674 - (view) | Author: Robert Collins (rbcollins) * (Python committer) | Date: 2014年09月10日 02:26 | |
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?
|
|||
| msg226693 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2014年09月10日 12:42 | |
Yeah, I figured out it was loader only errors after I read the code :) "load_tests not called" is very different from "load_tests produced an exception", so the text of the error message should be changed accordingly. I understood your use case more-or-less, my question was, why is the error *string* the thing returned? Given that the synthetic test encapsulates and re-raises the error, might it be more useful to store the exception object in the errors attribute rather than the message strings? That would seem to provide more introspectability. |
|||
| msg227318 - (view) | Author: Robert Collins (rbcollins) * (Python committer) | Date: 2014年09月23日 00:29 | |
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. |
|||
| msg227363 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2014年09月23日 14:55 | |
Oh, ok, if the existing glue does it that way, then it seems fine. I thought when I read the code that it was holding a reference to the traceback until it raised the error in the synthetic test. Or do you mean that when exceptions are raised by tests it is the string that is captured not the exception? That would make sense. So, yeah, I guess capturing the string makes sense, to be consistent with the rest of the reporting. |
|||
| msg227500 - (view) | Author: Robert Collins (rbcollins) * (Python committer) | Date: 2014年09月25日 01:26 | |
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. |
|||
| msg229704 - (view) | Author: Roundup Robot (python-dev) (Python triager) | Date: 2014年10月20日 00:24 | |
New changeset e906e23931fa by Robert Collins in branch 'default': Close #19746: expose unittest discovery errors on TestLoader.errors https://hg.python.org/cpython/rev/e906e23931fa |
|||
| msg250656 - (view) | Author: Peter (maubp) | Date: 2015年09月14日 11:38 | |
This comment is just to note that this change broke our (exotic?) usage of unittest.TestLoader().loadTestsFromName(name) inside a try/except since under Python 3.5 some expected exceptions are no longer raised. My nasty workaround hack: https://github.com/biopython/biopython/commit/929fbfbcf2d1ba65ec460a413128dd5e6f68f5bf I think it is unfortunate that the .errors attribute is just a list of messages as strings, rather than the original exception object(s) which would be easier to handle (e.g. using the subclass hierarchy). |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:57:54 | admin | set | github: 63945 |
| 2015年09月14日 11:38:46 | maubp | set | nosy:
+ maubp messages: + msg250656 |
| 2014年10月20日 00:24:38 | python-dev | set | status: open -> closed nosy: + python-dev messages: + msg229704 resolution: fixed stage: resolved |
| 2014年09月25日 01:26:27 | rbcollins | set | messages: + msg227500 |
| 2014年09月23日 14:55:41 | r.david.murray | set | messages: + msg227363 |
| 2014年09月23日 00:29:05 | rbcollins | set | messages: + msg227318 |
| 2014年09月10日 12:42:16 | r.david.murray | set | messages: + msg226693 |
| 2014年09月10日 02:26:24 | rbcollins | set | messages: + msg226674 |
| 2014年09月10日 01:36:22 | r.david.murray | set | nosy:
+ r.david.murray messages: + msg226671 |
| 2014年09月10日 01:11:06 | r.david.murray | set | files: + issue19746-rebased.patch |
| 2014年09月08日 21:27:16 | rbcollins | set | files:
+ issue19746.patch keywords: + patch messages: + msg226612 |
| 2014年04月08日 08:33:50 | vstinner | set | nosy:
+ vstinner |
| 2013年11月25日 05:02:10 | ncoghlan | set | title: No introspective way to detect ModuleImportFailure -> No introspective way to detect ModuleImportFailure in unittest |
| 2013年11月24日 20:33:58 | michael.foord | set | messages: + msg204265 |
| 2013年11月24日 03:16:03 | rbcollins | create | |