Code

Opened 6 years ago

Closed 11 months ago

#8363 closed New feature (wontfix)

Make it possible to specify tests to skip when running runtests.py

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

Description

Not a bug but a small enhancement to the test runner. It allows one to run, for example:

tests $ runtests.py --verbosity=2 -- -basic

to run all the Django test suite minus the modeltests/basic ones

or

tests $ runtests.py -- -model_forms -dispatch

to run all the Django test suite minus modeltests/model_forms and regressiontests/dispatch ones.

Attachments (5)

exclude_tests.diff (2.9 KB) - added by ramiro 6 years ago.
t8363-r9569.diff (8.4 KB) - added by ramiro 5 years ago.
Patch modified to use --exclude option and updated to r9569
8363-r12262.diff (4.9 KB) - added by ramiro 4 years ago.
Patch updated to r12262
8363-r14297.2.diff (7.3 KB) - added by ramiro 3 years ago.
8363.exclude-tests.diff (19.3 KB) - added by julien 3 years ago.

Download all attachments as: .zip

Change History (26)

Changed 6 years ago by ramiro

comment:1 Changed 6 years ago by mtredinnick

  • milestone set to post-1.0
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

That's a very bad syntax. Things start with a single dash are short-form options. Don't overload that (for example "-validate" is ambiguous, since -v is already used as an option).

An "exclude certain tests" is probably reasonable, but you probably need a proper option for it.

comment:2 Changed 6 years ago by russellm

The best option for syntax is to take the lead from #7254, and use an --exclude option.

comment:3 Changed 5 years ago by ramiro

  • Patch needs improvement set

Changed 5 years ago by ramiro

Patch modified to use --exclude option and updated to r9569

comment:4 Changed 5 years ago by ramiro

  • Patch needs improvement unset
  • Summary changed from Make runtests.py able to run all tests but the ones specified by prefixing its names with '-' to Make it possible to specify tests to skip when running runtests.py

comment:5 Changed 5 years ago by ramiro

  • Owner changed from nobody to ramiro

comment:6 Changed 5 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:7 Changed 5 years ago by ramiro

Eric Hoslscher correctly notes in a comment to #4501 that the Django simple test runner run_tests() function is an exposed API so adding a parameter like the patch does with the exclude_labels parameter would introduce a backwards-incompatibility for third party test runners.

Changed 4 years ago by ramiro

Patch updated to r12262

comment:8 Changed 4 years ago by ramiro

I don't know is this is worth adding to the Django test infrastructure or belongs to a custom runner but has been very helpful when using the Django test suite to so I thought it could also be useful to others.

comment:9 Changed 4 years ago by ericholscher

  • milestone set to 1.3

Changed 3 years ago by ramiro

comment:10 follow-up: Changed 3 years ago by ericholscher

Responded to ramiro on IRC, but putting my thoughts here too to have a better record.

Seems that this should be exposed to the user-facing test management command as well. I think that having as little special in runtests, and keeping feature parity there is a good goal. This also means that appropriate documentation should be added.

Seems that there should also be some tests, at least running through the code-path where the suite is built, and making sure that excluded things aren't in that suite.

Otherwise this looks like a big win.

comment:11 in reply to: ↑ 10 Changed 3 years ago by ramiro

Replying to ericholscher:

Responded to ramiro on IRC, but putting my thoughts here too to have a better record.

Seems that this should be exposed to the user-facing test management command as well. I think that having as little special in runtests, and keeping feature parity there is a good goal. This also means that appropriate documentation should be added.

Problem is that currently building of the test suite is delegated to a tightly cooperation of the Django app cache and unittest2 (http://code.djangoproject.com/browser/django/trunk/django/test/simple.py?rev=14696#L209). *

In the test management command case, the app cache is populated from INSTALLED_APPS and we don't have (yet) an API to get app names (the ones we would get form the command line) from app objects (the ones we are handling in django.db.models.get_apps()), we need this to hook there and skip addition of an app to the suite.

Also, unittest[2] doesn't have an API to remove tests from a suite once it is built and before running it.

In runtests.py we have more control over what apps from tests/regressiontests and tests/modeltests are loaded and can control the suite contents by simply avoiding the loading of the apps we want excluded.

AFAICT this mean it would be practical to only add this feature to the runtests.py script.

  • This lack of granularity is the same root cause that keep us from excluding test cases or test methods; so the syntax supported by the --exclude switch will only allow the user to exclude entire test apps from the test run.

comment:12 Changed 3 years ago by lukeplant

  • Severity set to Normal
  • Type set to New feature

comment:13 Changed 3 years ago by patchhammer

  • Easy pickings unset
  • Patch needs improvement set

8363-r14297.2.diff fails to apply cleanly on to trunk

comment:14 Changed 3 years ago by julien

  • UI/UX unset

So I've spent a bit of time working on this. The attached patch implements full support for the --exclude option both for the test and runtests commands. To do so I've had to refactor the DjangoTestSuiteRunner quite heavily following an approach that one may not find appropriate, so I'd welcome some feedback.

In a nutshell, the suggested approach is to generate a one-level-deep dictionary: label->test, where label is a full string reference to a test (of the form app.TestClass.test_method) and test is either a test case or a doctest instance. For example:

{
    'auth.AnonymousUserBackendTest.test_get_all_permissions':
        <django.contrib.auth.tests.auth_backends.AnonymousUserBackendTest testMethod=test_get_all_permissions>,
    'auth.AnonymousUserBackendTest.test_has_module_perms':
        <django.contrib.auth.tests.auth_backends.AnonymousUserBackendTest testMethod=test_has_module_perms>,
    ...
}

This then makes it trivial to exclude some given tests before running the test suite by removing the dictionary items corresponding to the excluded tests' labels. However, there is a concern: using this flat structure means that some information may get lost in the process (e.g. the nesting of the various sub-suite instances) which may potentially introduce some annoying limitations in the future.

By the way, about 13 tests out of 4065 don't seem to be run after this patch. I need to narrow down the cause of this. Another note: it seems bizarre that, even in trunk, both unittest.TestLoader and unittest.defaultTestLoader are used in simple.py.

Changed 3 years ago by julien

comment:15 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

comment:16 Changed 2 years ago by mitar

  • Cc mmitar@… added

I would really like to see this also a settings options, like AdvancedTestSuiteRunner has.

comment:17 Changed 22 months ago by ptone

julien,

this would be pretty handy - but the patch is pretty stale an no longer applies cleanly.

I'm trying to debug some testsuite interactions - and there are a couple tests I'd like to take off the table for a bit and come back to.

I found the best workaround is just to remove the module directories from modeltests/regressiontests temporarily - yuck.

comment:18 Changed 22 months ago by julien

I think this would actually need some deeper refactoring of the test discovery system to make it work more like other frameworks (e.g. pytest, nose). I think carljm and alex have got some initial thoughts on that. It would be a pretty large effort, but one that would solve many problems at once by doing things right.

comment:19 Changed 13 months ago by aaugustin

This will most likely be closed as a duplicate of #17365 at some point.

comment:20 Changed 13 months ago by carljm

The plan for #17365 is basically to make "manage.py test" work just like "python -m unittest discover", and avoid custom Django test-discovery code. As far as I know unittest does not provide the feature requested here, so closing this as a duplicate of #17365 would be essentially the same as closing wontfix. This is personally fine with me, but a number of other core devs here have commented that they find this useful.

py.test does provide this feature (via the -k option with a - prefixed keyword). I think switching to py.test for testing Django itself would be a legitimate option; I'd much rather do that than add more custom Django test-runner code. This would be a separate thing from #17365.

comment:21 Changed 11 months ago by aaugustin

  • Resolution set to wontfix
  • Status changed from new to closed

I understand.

Like you, I believe the way forwards is to make it possible to run Django's tests with third-party test runners, not to extend our custom test runner. Although some core devs have said they'd find this useful, none of them have found it sufficiently useful to implement it.

I'm going to close the ticket because of the new direction our test discovery is taking — not to prevent anyone from implementing it, just to stop tracking an increasingly irrelevant feature request.

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.