On Mon, Sep 27, 2010 at 6:24 AM, Karen Tracey <[email protected]> wrote: > I've run it on Python 2.4 & 2.5 (Ubuntu, sqlite DB) with no problems. > > I do have some feedback on the @skipIfDB addition: I'd really like if this > could be used to distinguish between the different MySQL storage backends. > From a very brief look it seems like currently there are a bunch of tests > skipped when the DB backend is MySQL, under the assumption that MySQL does > not have transaction support. However MySQL does have transaction support > when the InnoDB storage engine is used, so it would be nice if these tests > were only skipped when the MySQL/MyISAM combination were in use, not when > MySQL/InnoDB was used. > > Similarly there are a bunch of tests that fail or produce errors when the > MySQL/InnoDB combination is in use, because that combination does not > support forward foreign-key references when loading fixtures. It's not ideal > to skip a bunch of tests for function that really should be tested on this > combination (aggregates, for example), but for now at least the fixtures > used by these tests do not allow them to pass on MySQL/InnoDB, so they might > as well just be skipped. I have quit ever trying to run the full Django test > suite using MySQL/InnoDB because there are just too many "known failures" > there. It would be nice if the addition of @skipIfDB improved that. (For > that matter I also never run the full suite with MySQL/MyISAM because the > lack of transaction support with that combo makes it just too slow to run > the full suite, but I don't know of any way to improve that situation.) > > One difficulty in adding this extra check is that it is hard to know for > sure what storage engine is in use. If the database definition dict includes > 'OPTIONS': { "init_command": "SET storage_engine=<whatever>" } then we know > the <whatever> engine is in use. If not, the default configured engine for > the MySQL server will be used, and that could be either one. I'd be inclined > to say if you want to skip tests based on a particular storage engine being > used, then you need to make it explicit in the test settings what engine you > are using. So I'd be happy if this @skipIfDB threw an error if it was asked > to skip based on storage engine but could not find in the test settings what > storage engine was being used.
The problem with this approach is that you can configure MySQL to use InnoDB by default for table creation, so there won't necessarily be any Django settings providing evidence that transactions are available. > Another difficulty is figuring out how to nicely specify this additional > requirement. Have not given that a whole lot of thought yet. Right now the > @skipIfDB takes a simple string (matched against DB ENGINE key) or iterable > of strings. Supporting querying both ENGINE and OPTIONS, and allowing for > more than one to be listed, might get a bit ugly. I am curious why the > iterable option here was added -- are there a lot of cases in the current > suite where tests need to be skipped on a set of DB backends and not just > one? The use case for the iterable is everywhere that PostgreSQL is the skip target, because we have 2 backends that need to be skipped. I'm in agreement that the ENGINE-string based skipIfDB isn't a particularly robust test. If nothing else, it means that if a user writes a third-party engine for MySQL (or anything else) in order to change some subtle behavior, the test suite won't be any use to them, because the string match won't cause the appropriate tests to be skipped. One thought here would be to introduce a variable on the backend itself that is a 'db identifier' - i.e., a string that clearly identifies the vendor, independent of the path that has been used to install the backend. So, for example, a check for connection.db_identifier = 'postgresql' would always tell you that you're dealing with PostgreSQL, regardless of the import path to the actual backend. This has three potential benefits. 1. We can avoid the need for the iterable skipIfDB version. 2. We enable writers of third-party backend for Django supported databases to benefit from test skip annotations 3. We provide a name that can be used for backend-specific features, such as the index-hinting proposal that has been floating around. However, this still doesn't address the InnoDB problem, or provide any support for completely external backends -- for example, the writers of the DB2 backend can't use Django's test suite, because the DB2 backend isn't specifically named in Django's source code (nor should it). It also occurs to me that in most cases, the skip should actually be based on capabilities, not on the backend itself. We should be skipping because the backend doesn't support transactions, or because the backend doesn't allow forward references in fixtures, not because the backend is MySQL. connection.features already provides one place to check whether certain features are available. It would be a relatively simple change to modify the test skips to check connection.features, rather than the ENGINE. This would have the added benefit that writers of backends that are not officially supported (e.g., DB2) would be able to skip the appropriate tests based purely upon the capabilities of their backend. Looking forward, this will be especially important when we get to supporting NoSQL backends. There are a couple of places that are literally "check that Oracle does this right, in an Oracle specific way", but they're the exception, not the rule. Opinions? Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
