*Progress.* - Deprecated `requires_model_validation` flag and `validate` method (both `BaseCommand` members) in favour of new `requires_system_checks` flag and `check` method.
- Finished working at `contenttypes` tests. - Improved code thanks to Preston Holmes comments. Deleted dead code and added some new tests to improve code coverage. It'd be nice to have checks of clashes between GenericForeignKey and accessors. I didn't implemented it because little time left and I need to hurry up. When it was easy, I've added new tests to improve code coverage. However, there are still some tests that would be nice to have: - Test for raising an error while using an unique varchars longer than 255 characters under MySQL backend. [1] - Test for `ImageField`. When `Pillow` is not installed and `ImageField` is in use, an error should be raised. This should be tested. [2] - Test for raising warning/ImproperlyConfigured in `BaseCommand.__init__`. [3] [1] https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/django/db/backends/mysql/validation.py#L6 [2] https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/django/db/models/fields/files.py#L447 [3] https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/django/core/management/base.py#L202 *Filtering and silencing errors/warnings.* We need two features: ability to run only a subset of checks (aka filtering errors) and ability to silence some errors. Silencing is easy. Every warning will have a unique name like "W027". The format of the name is letter "W" followed by a unique number. The system check framework is open ended and third-party apps can register its own checks. For warnings raised by these apps, "Wnumber.applabel" (e.g. "W001.myapp") style will be used. Of course, everything can be changed and I'm open to yours suggestions, so feel free to comment and criticize it. There will be a new setting called `SILENCED_WARNINGS`. If you want to silence a warning, you put its name in this setting, e.g.: SILENCED_ERRORS = [ 'W027', ] Only light messages (warnings, infos and debugs) can be silenced. Running only a subset of check is a more complex task. We can associate every check with a set of tags (that can be done while registering checks). To run only checks tagged "mytag" or "mysecondtag", you need to type: manage.py check mytag mysecondtag However, we would like to run checks of an app (or a set of apps): manage.py check auth admin This is hard, because checks are no longer app-specific. I propose to solve this by passing an `apps` optional keyword argument to every check function. The function should only validate specified apps (or all apps if apps==None). The only problem is how to determine if we deal with a tag or app name? Consider this command: manage.py check auth mytag This should run all checks tagged "mytag". Only messages for `auth` app should be reported. I propose to collect all tags while registering check functions and if a string is one of them, then interpret it as a tag, otherwise assume it's an app name (and check if an app with given name exists). *Problems/questions.* 1. We decided to mimic message and logging framework, so every error is tagged with a level. Its value is an integer indicating how important and serious is a message. There are five predefined values: CRITICAL, ERROR, WARNING, INFO, DEBUG. However, Preston Holmes noticed that this is unpractical because we need only errors and warnings. I think we should discuss again if it's worth mimicing these frameworks. 2. On the pull request we started discussing names. "System check" is better than "check" but it suggest that it's connected with hardware. Preston proposed "Startup checks". I don't have strong opinion. To be honest, I don't thing "startup checks" is much more better than "system checks" so I will leave it as it is, but I'm still open to suggestions and I would like to see yours opinion. 3. There are some problems with moving custom User model checks. Their first source was that some tests override `INSTALLED_APPS` setting but don't override list of registered checks. So checks of `auth` app were registered (because the app was imported by some other tests), but this app wasn't installed. This ended in an error, because checks of `auth` try to load User model which is, by default, `auth.User` and `auth` is not installed. I've solved this by overriding list of registered checks (I've introduced `override_system_checks` decorator). However, there is still one red test -- `admin_scripts.test_complex_app` [4]. The problem is that this test spawns a new Django process and the decorator cannot affect this new process. This test installs two apps (`complex_app` and `simple_app`) and doesn't install `auth` app. However, `auth` app is imported, because `complex_app` imports `admin` app which imports `auth` app. The simplest solution is just to add `auth` app to `INSTALLED_APPS`. [4] https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/tests/admin_scripts/tests.py#L1078 This shows a drawback of the current registration approach. Every time an app is imported but not installed, its checks are registered, but they shouln't be. I think we should rethink how checks should be registered. An alternative can be Russell's solution -- create a new setting `SYSTEM_CHECKS` containing list of all checks. This has one small drawback -- when you want to install an app with custom checks, you need to put it in both `INSTALLED_APPS` and `SYSTEM_CHECKS`; but we can overcome it by implicit extending `SYSTEM_CHECKS` with app-specific checks. So, I can write: INSTALLED_APPS = [ 'auth', ] And all checks of `auth` app will be automagically registered. Of course, we need some public API for retrieving app checks. We can reuse the existing registering mechanism and add `app` argument to `checks.register` function indicating that this one particular check should be run only if app `app` is installed. 4. In my last post I mentioned that there is a problem with `BaseCommand.verbosity`. I tried to convert its default value ('1') into an unicode, but I failed. This test command [5] prints its arguments to stdout, so the output is something like this: ... options=[('pythonpath', None), ... ('verbosity', '1')] But when you change `verbosity` from a bytestring to an unicode then the output is (only under Python 2): ... options=[('pythonpath', None), ... ('verbosity', u'1')] Russell proposed to replace `sorted(options.items())` in [5] with: sorted((text_type(k), text_type(v)) for k, v in options.items()) This is a solution, but it makes tests less strict. There is no way to distinguish between `None` value and "None" string -- both are printed in the same way. I spent some time to find a better approach but with no success. The least insane way is to check if the object to print is an unicode and if it is, print it in the Python 3 style (without starting "u"); otherwise print it normally. Note that this is a more general problem. I've hit the same problem when I tried to write a test for `CheckMessage.__repr__` method. I wrote: def test_repr(self): e = Error("Error", hint="Hint", obj=None) expected = "<CheckMessage: level=40, msg='Error', hint='Hint', obj=None>" self.assertEqual(repr(e), expected) But under Python 2, `expected` variable should be: expected = "<CheckMessage: level=40, msg=u'Error', hint=u'Hint', obj=None>" [5] https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/tests/admin_scripts/management/commands/base_command.py#L17 5. I've deleted one filter in [6]. I think it's useless. If `self.related.get_accessor_name` is None, then `self.rel.is_hidden()` is true, so this is a dead if. Am I right? [6] https://github.com/chrismedrela/django/commit/be54f9411526c660e93ae67cbcddbd173a02b81a 6. When `ForeignKey.foreign_related_fields` is a list of more than one element? If it's always a one-element list, then we don't need lines 1443-1457 in `ForeignKey._check_unique_target`. [7] [7] https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/django/db/models/fields/related.py#L1443 7. It looks like the code I've deleted in this commit [8] was dead, because if you refer to an inherited field, an FieldDoesNotExist exception is raised by `get_field` method. Is that right? [8] https://github.com/chrismedrela/django/commit/e41ae69a75d85c0325a61fdcf00cd03ce1692913 8. I've added a new check. If you're using a `GenericRelation` but there is no `GenericForeignKey` on the target model, an warning is raised. This check was implemented in this commit [9]. It uses `vars` builtin function to check if the target model has a `GenericForeignKey`. This is ugly, but I don't see a better approach. [9] https://github.com/chrismedrela/django/commit/ab65ff2b6d6346407a11a72c072e358c7b518cf9#L1R397 *Schedule.* There is little time to the end of GSoC -- only 5 weeks and I will work 50% of full speed after September 6, because I'm going on holiday. - Rewriting admin checks (1 week). - Implementing filtering and silencing errors (1 week). - Merging django-secure (2 weeks?!). - Polishing doc, last review (1 week). I will implement filtering and silencing errors before merging django-secure because these are a must if we want to merge django-secure. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.