1. Regardless of the discovery logic in `get_test_modules`, you can always
run any of these tests explicitly by providing the test label, and if you
do so with the wrong database, the result should be skipped tests (with a
reason for the skip which the test runner can output at the appropriate
verbosity level), not failing tests (which could be quite confusing).
2. The overall number of tests in Django's test suite (when no test labels
are provided) should not vary depending on your settings, some tests
should just be marked as skips. This makes it explicit to someone running
the tests with a non-Postgres database (for instance) that there are a
number of Postgres-specific tests which are being skipped for them, rather
than silently hiding those tests entirely.
--
Ticket URL: <https://code.djangoproject.com/ticket/23879>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* version: 1.6 => master
* stage: Unreviewed => Accepted
Comment:
In fact I think that `django.contrib.gis` tests already satisfy your
demand. If someone has a GIS-lib-free system at hand, it would be nice to
test and report any failure/traceback.
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:1>
Comment (by carljm):
Yes, that's right. I fixed one issue in
4932a7b8e0a25c7447d7ad7ae8ae53a35d2541ac yesterday. I have a system here
that doesn't have GEOS installed, and I can run `runtests.py
django.contrib.gis` and get a bunch of skipped tests, some passed tests,
no failures or errors.
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:2>
Comment (by carljm):
Hmm, I'll need to look into this a bit more. The issue is not with the
tests themselves, but with the test models/migrations; in at least the GIS
case, those fail immediately when creating the test database if you don't
have a GIS-enabled database backend.
I know we had this working when the test-discovery branch was merged; I'll
need to look back and see how it worked then and what's changed since.
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:3>
Comment (by prestontimmons):
Carl,
I think the discovery result is still the same as it was when the discover
runner was merged. Claude's changes simply replaced the old `geo_apps`
function:
https://github.com/django/django/commit/bd563f0f571e7c76b40e8c8d7a0e1f34dcfeb810
https://github.com/django/django/commit/57f190ed3b77eb105cc86ecfabc455662410d40a
Using `discovery_paths` this way—to conditionally add test applications
from contrib to INSTALLED_APPS—isn't really obvious from the code, though.
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:4>
Comment (by wrwrwr):
I can confirm that removing the GIS toggle already gives a bunch of
skipped tests without any failures or errors.
On the other hand, postgres tests can't be dealt with easily --
`postgres_tests` have some migrations that rely on PostgreSQL specific
operations, and cause the test database creation to fail with any other
backend. Resolving this would require some way to limit migrations or
whole test apps to specific backends.
I was able to get it to work with an
[https://github.com/wrwrwr/django/compare/feature/always-test-gis-and-
postgres ad-hoc solution] (that wasn't properly thought over or
researched). It may be an interesting question how to deal with database-
specific migrations in reusable apps.
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:5>
Comment (by timgraham):
That looks interesting. It would probably be worth discussing the issue of
database vendor specific migrations on the DevelopersMailingList. If it's
been discussed before I'm not aware of it.
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:6>
Comment (by claudep):
Wojtek, I think your POC patch is interesting, and would be worth being
completed with docs and tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:7>
Comment (by carljm):
I agree. I think there are probably some aspects that require further
thought (like what happens if a later migration is "appropriate for this
connection" but has a dependency on one that is not?), but I think the
general idea is sound.
I wonder if perhaps it would work better if it were operations rather than
entire migrations that were marked as for-certain-db-vendor-only? I could
imagine some API like a mark-for-vender wrapper to surround a list of
operations in a migration file.
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:8>
Comment (by MarkusH):
The general idea to have some vendor specifics in the migration code base
sounds good. But I have to agree with Carl on the problems that arise from
not loading a migration. I think the skipping of vendor miss-matches
should happen during the actual migrate process. This still allows
dependency resolutions and state changes independent from the vendor.
Moving the vendor part into operations would furthermore give us some
flexibility for advanced database features. E.g. once `contrib.postgres`
got support for views, these views could be generated within the same
migration, one operation could be
`django.contrib.postgres.operations.CreateView` (with a PostgreSQL vendor
marker) while a `RunSQL` operation would run everywhere but on PostgreSQL.
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:9>
* has_patch: 0 => 1
Comment:
I finally succeeded in having a seemingly-working patch. Not commit-ready,
but a base for discussion:
https://github.com/django/django/pull/4452
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:10>
* needs_better_patch: 0 => 1
Comment:
Seems on the right track to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:11>
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:12>
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:13>
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"8097e5483210fa8c9d6142f1d48e1caed6d7b7bb" 8097e54]:
{{{
#!CommitTicketReference repository=""
revision="8097e5483210fa8c9d6142f1d48e1caed6d7b7bb"
Fixed #23879 -- Allowed model migration skip based on feature/vendor
Thanks Carl Meyer for the report and review, and Tim Graham for the
review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:14>
Comment (by Claude Paroz <claude@…>):
In [changeset:"6b6d13bf6ed02c345912829e3850a201f113712a" 6b6d13b]:
{{{
#!CommitTicketReference repository=""
revision="6b6d13bf6ed02c345912829e3850a201f113712a"
Stopped conditional discovery of gis_tests apps
Refs #23879.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:15>
Comment (by Claude Paroz <claude@…>):
In [changeset:"36e90d1f45a13f53ce25fdc2d9c04433b835c9af" 36e90d1f]:
{{{
#!CommitTicketReference repository=""
revision="36e90d1f45a13f53ce25fdc2d9c04433b835c9af"
Stopped special-casing postgres-specific tests
Refs #23879.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:16>
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"2fb872e56f064361c2cd4674c0f23cd419994d82" 2fb872e5]:
{{{
#!CommitTicketReference repository=""
revision="2fb872e56f064361c2cd4674c0f23cd419994d82"
Refs #23879 -- Made introspection respect required_db_features.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23879#comment:17>