Code

Opened 3 years ago

Closed 10 months ago

#16534 closed Cleanup/optimization (fixed)

DiscoverRunner API makes re-use difficult.

Reported by: tomchristie Owned by: jcd
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: andy@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Would be better if it allowed the test suite class and test runner class to be overridden.

Attachments (6)

djangotestsuiterunner.diff (1.3 KB) - added by tomchristie 3 years ago.
djangotestsuiterunner-2.diff (4.0 KB) - added by tomchristie 3 years ago.
Also allow test_runner to be overridden
djangotestsuiterunner-3.diff (3.0 KB) - added by jcd 10 months ago.
New patch that works with DiscoverRunner instead of DjangoTestSuiteRunner
djangotestsuiterunner-4.diff (4.7 KB) - added by jcd 10 months ago.
Added documentation updates suggested on IRC by timograham
djangotestsuiterunner-4.diff​ (4.7 KB) - added by jcd 10 months ago.
Added documentation updates suggested on IRC by timograham (edited to fix a documentation typo: DicsoverRunner)
discoverrunner-with-tests.diff (5.7 KB) - added by jcd 10 months ago.
One last attachment, this one with tests.

Download all attachments as: .zip

Change History (24)

Changed 3 years ago by tomchristie

comment:1 Changed 3 years ago by tomchristie

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by jezdez

  • Needs documentation set
  • Needs tests set

Sounds like a reasonable idea.

comment:3 Changed 3 years ago by jezdez

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 3 years ago by tomchristie

Being able to override the test loader too would be even better...

Changed 3 years ago by tomchristie

Also allow test_runner to be overridden

comment:5 Changed 3 years ago by Andy Baker <andy@…>

  • Cc andy@… added
  • Needs documentation unset
  • Needs tests unset

Don't think this needs documentation - it's an internal interface, and don't think it needs tests as it's clean-up, not new functionality.

comment:6 Changed 3 years ago by tomchristie

Hadn't noticed those were still set. Thanks.

comment:7 follow-up: Changed 3 years ago by ramiro

  • Needs documentation set

DjangotestSuiteRunner is documented, changes introduced for this ticket also need documentation.

comment:8 in reply to: ↑ 7 Changed 3 years ago by tomchristie

Replying to ramiro:

DjangotestSuiteRunner is documented, changes introduced for this ticket also need documentation.

Ah, true. I'll get that sorted, then...

comment:9 Changed 14 months ago by tomchristie

Don't know how this patch applies given 1.6's incoming test runner changes, if anyone's picking this up at DjangoCon, that'd need investigation first...

comment:10 Changed 10 months ago by jcd

I'm taking a look at this....

comment:11 Changed 10 months ago by jcd

So it doesn't apply as is. I've modified to to work with the new subclassing of DiscoverRunner. All tests still pass. I'm looking into the documentation situation, and then I'll be uploading a patch.

Changed 10 months ago by jcd

New patch that works with DiscoverRunner instead of DjangoTestSuiteRunner

comment:12 Changed 10 months ago by jcd

  • Owner changed from tomchristie to jcd
  • Status changed from new to assigned

Looking more closely, DjangoTestSuiteRunner is deprecated. Updating it seems like the wrong way to move forward. Instead, we should ensure that the new hotness (DiscoverRunner) contains the requested features.

It already allows overriding test_loader. Following the example in previous patches, I have updated DiscoverRunner to allow overriding the test suite and the unittest-level test runner (TextTestRunner). I have also updated the documentation of DiscoverRunner at topics/testing/advanced/ to detail these attributes.

In a small departure from earlier patches, I have taken the liberty of renaming suite_class to test_suite (for consistency with test_runner and test_loader). Because it is pending deprecation, no enhancements have been made to django.test.simple or the DjangoTestSuiteRunner.

comment:13 Changed 10 months ago by jcd

  • Needs documentation unset

Changed 10 months ago by jcd

Added documentation updates suggested on IRC by timograham

comment:14 Changed 10 months ago by jcd

  • Version changed from 1.3 to master

comment:15 Changed 10 months ago by jcd

  • Summary changed from DjangoTestSuiteRunner API makes re-use difficult. to DiscoverRunner API makes re-use difficult.

Changed the ticket title to reflect the new direction of the ticket.

Changed 10 months ago by jcd

Added documentation updates suggested on IRC by timograham (edited to fix a documentation typo: DicsoverRunner)

Changed 10 months ago by jcd

One last attachment, this one with tests.

comment:16 Changed 10 months ago by jcd

I added tests. They're pretty brain dead, because the entire change was an attribute creation refactor, but now that the attributes are documented, we want to be notified if they go away or become the wrong thing.

comment:17 Changed 10 months ago by jcd

Created a ticket to track backporting some documentation changes to 1.6.

comment:18 Changed 10 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 4ba373840a7b299fff4a7bcc1001e32dffceee86:

Fixed #16534 -- Improved ability to customize DiscoverRunner

Added DiscoverRunner.test_suite and .test_runner attributes.

Thanks tomchristie for the suggestion and jcd for the patch.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.