search

yunojuno / django-juno-testrunner

A more useful (and slightly more glamorous) test runner for Django 1.6+ from the folks at YunoJuno
MIT License
7 stars 9 forks source link

Circumvent invalid test names caused by import errors #23

Closed gergelypolonkai closed 8 years ago

stevejalim commented 8 years ago

Thanks, @gergelypolonkai!

As mentioned before, a test for the regression-warning test in the custom loader would be nice, if you've still got it around.

gergelypolonkai commented 8 years ago

You mean the case if unittest decides to put the ImportError exception to a different path? Or a specific warning I don’t see in my logs? (If the latter, I’d be thankful for a log excerpt!)

stevejalim commented 8 years ago

The former, I reckon - but it’s a small thing

Steve Jalim

Technical Director

steve@somefantastic.co.uk // 07740 819849 // 020 3239 3292

www.somefantastic.co.uk // @SomeFantastic http://twitter.com/SomeFantastic

Some Fantastic Limited, 51 Somers Park Avenue, Malvern, Worcestershire WR14 1SE, UK. Registered in England, No. 06194144 // VAT reg: GB 118051641

On 18 August 2016 at 14:03, Gergely Polonkai notifications@github.com wrote:

You mean the case if unittest decides to put the ImportError exception to a different path? Or a specific warning I don’t see in my logs? (If the latter, I’d be thankful for a log excerpt!)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/yunojuno/django-juno-testrunner/pull/23#issuecomment-240716293, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGMUWcj2iHWWAF5GeXG6Cgx0KGuEcXlks5qhFgsgaJpZM4JndWJ .

gergelypolonkai commented 8 years ago

I had a proposal in #18 for that. Or we can look for ModuleImportFailure in the test case name, but if the exception gets renamed, we’re doomed again… Maybe we can run a test discovery (:warning: overhead alert! :warning:) and compare test names to the ones in the rerun log, but that sounds much worse than relying on unittest’s API.

Speaking of which, I find it pretty unlikely this exception will be renamed or moved. unittest API seems to be stable for a bunch of major releases now, so I don’t see the point.

stevejalim commented 8 years ago

Sure - let’s go without it. If the code moves beneath it, we'll know soon enough :)

@Geekfish - you like?

Geekfish commented 8 years ago

Looks like a quick win 👍

gergelypolonkai commented 8 years ago

Just to note, this PR will solve #1 (just to add a link between the original issue and the PR).