[Django] #32178: Allow database backends to skip tests and mark expected failures
Django
-------------------------------------+-------------------------------------
Reporter: Tim | Owner: nobody
Graham |
Type: New | Status: new
feature |
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Currently Django's built-in database backends have special privileges in
the test suite when it comes to test skipping such as:
`tests/model_fields/test_integerfield.py:
@unittest.skipIf(connection.vendor == 'sqlite', "SQLite doesn't have a
constraint.")`
While `DatabaseFeatures` attributes are generally preferred to
`connection.vendor` checking, I think allowing backends to skip certain
tests has its place. Adding features flags that are likely to apply to
only a single database is a bit cumbersome, especially for third-party
backends.
Django should provide more generic way to skip tests and mark expected
failures. For example, see [https://github.com/cockroachdb/django-
cockroachdb/blob/6304e41fc5725fcfe53b851498529c339ba89f20/django_cockroachdb/creation.py#L17-L214
django-cockroachdb's logic] which was inspired by
[https://bitbucket.org/Manfre/django-
mssql/src/master/sqlserver_ado/creation.py django-mssql-backend]. In a
more refined version, `expected_failures` and `skip_classes` would be
attributes of `DatabaseFeatures`.
That logic uses a `RUNNING_COCKROACH_BACKEND_TESTS` environment variable
to decide whether or not the Django test suite is running. There might be
a better way to determine that. At least the variable name would be more
generic.
--
Ticket URL: <https://code.djangoproject.com/ticket/32178>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:1>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Hasan Ramezani):
> In a more refined version, `expected_failures` and `skip_classes` would
be attributes of DatabaseFeatures.
What can we do to skip a single test? I checked the `django-cockroachdb`
and there you have a `skip_tests` which can be used to skip a single
test(If I got it right).
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:2>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
I implemented single test skipping in [https://github.com/googleapis
/python-spanner-
django/blob/05a1640ab0b295a18eb0f99246dcacd4bce969b7/django_spanner/creation.py#L17-L30
python-spanner-django].
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:3>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Hasan Ramezani):
Hi Tim,
I created a [https://github.com/django/django/pull/13717 PR] based on
`django-cockroachdb` example. Please check the PR if you have time and if
you confirm it, I will continue with moving skip tests to database
creation files.
I added `expected_failures_tests` and `skip_tests`(which can be used for
skipping both test class and function) and added a `mark_tests` method
similar to the one that you did for `django-cockroachdb`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:4>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Hasan Ramezani
* status: new => assigned
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:5>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
I'll mark this as RFC when I'm done iterating with Hasan. (Meanwhile,
let's leave "needs improvement" checked so it doesn't appear in the review
queue for others.)
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:6>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:7>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:8>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:9>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:10>
Django
-------------------------------------+-------------------------------------
| Ramezani
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"275dd4ebbabbbe758c7219a3d666953d5a7b072f" 275dd4eb]:
{{{
#!CommitTicketReference repository=""
revision="275dd4ebbabbbe758c7219a3d666953d5a7b072f"
Fixed #32178 -- Allowed database backends to skip tests and mark expected
failures.
Co-authored-by: Tim Graham <timog...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:11>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"3f140dde51c0fe6c350acb7727bbe489a99f0632" 3f140dde]:
{{{
#!CommitTicketReference repository=""
revision="3f140dde51c0fe6c350acb7727bbe489a99f0632"
Refs #32178 -- Changed BaseDatabaseFeatures.django_test_expected_failures
to set().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:12>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"1e7653117f273391705b10c78214e27703e484b8" 1e765311]:
{{{
#!CommitTicketReference repository=""
revision="1e7653117f273391705b10c78214e27703e484b8"
Refs #32178 -- Fixed test_mark_expected_failures_and_skips_call teardown.
Test isolation failure observed on CockroachDB and PostgreSQL.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:13>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Chris Jerdonek):
I have a comment / question about this feature. I was analyzing a test by
inspecting the code, and I couldn't figure out why it was passing on
SQLite because it used to have "skip" code which is no longer there. And
then I came across this ticket via the history.
Would it be possible to have at least a code convention of including a
code comment at the test site when the test is included in
`django_test_skips`? Otherwise, the average reader wouldn't know to check
there for the test's name. (I would call this behavior "magical" because
it's not explicit at the code site.) Also, even for people that know about
it, it's not something that's easy to check while e.g. browsing the code
on the web.
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:14>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
I sympathize but would you give third-party backends the same privilege?
If so, it may be better to scrap this feature and allow third-parties to
contribute their skips directly in the test suite. If not, this feature
could at least be documented in the contributing guide.
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:15>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: closed
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Hasan Ramezani):
I've created a [https://github.com/django/django/pull/14317 PR] to add
them to the contributing guide.
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:16>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:17>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"ca34db46504fca1221e27f6ab13734dfdfde6e1c" ca34db46]:
{{{
#!CommitTicketReference repository=""
revision="ca34db46504fca1221e27f6ab13734dfdfde6e1c"
Refs #32178 -- Doc'd
DatabaseFeatures.django_test_skips/django_test_expected_failures in
contributing guide.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:18>
Django
-------------------------------------+-------------------------------------
| Ramezani
Type: New feature | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"ce130749d51302006f76e33b9987eaec20589837" ce130749]:
{{{
#!CommitTicketReference repository=""
revision="ce130749d51302006f76e33b9987eaec20589837"
[3.2.x] Refs #32178 -- Doc'd
DatabaseFeatures.django_test_skips/django_test_expected_failures in
contributing guide.
Backport of ca34db46504fca1221e27f6ab13734dfdfde6e1c from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32178#comment:19>