DiscoverRunner's setup_test_environment() hard-codes settings.DEBUG to False

[Chris Jerdonek]

Currently, the setup_test_environment() method of Django's DiscoverRunner hard-codes setting settings.DEBUG to False. See ​here for a direct link to that code.

This is less convenient for subclassers that would like to provide the ability to selectively run certain tests with DEBUG set to True (e.g. see here for a ticket to expose this as a command-line option).

This ticket is to make subclassing easier by adding a debug_mode instance attribute to DiscoverRunner (the suffix "mode" is to better distinguish from the current debug_sql attribute). This would also make progress towards ticket #27008.

[Tim Graham]

Did you consider an argument to DiscoverRunner.setup_test_environment() instead? It's not clear to me that subclassing the test runner is the best approach for #27008.

[Chris Jerdonek]

This issue is just an easy subset of what would be done for #27008 anyways. But it has the nice feature that it would make it easier for others to add a --debug option on their own via subclassing even before #27008 is closed.

In other words, I wasn't saying #27008 would be addressed via sub-classing. Rather, it would allow others to subclass in lieu of the issue.

In terms of your suggestion, adding an argument to DiscoverRunner.setup_test_environment() would just push the issue back another method to DiscoverRunner.run_tests(), where setup_test_environment() is called (in which case subclassers would have to reimplement all of run_tests() just to change how setup_test_environment() is called). All of the other command-line options work by setting an instance attribute in the constructor and using those attributes throughout the methods, which is what I'm suggesting doing here.

[Chris Jerdonek]

Or are you suggesting that an argument be added to both DiscoverRunner.setup_test_environment() and DiscoverRunner.run_tests()? That would be another way to address the subclassing issue without needing to copy / reimplement methods.

See ​here for how the test management command constructs the runner and invokes run_tests(). As you can see, there are two options for passing options: passing to the constructor and passing to run_tests(). Currently, only the test labels are being passed to run_tests() and the rest are passed via the constructor.

[Tim Graham]

Following the pattern of the other option in DiscoverRunner.__init__() seems fine, but do you have a subclassing use case in mind?

[Chris Jerdonek]

Yes, to add a --debug option more easily prior to ticket #27008 being addressed. :) But that is all.

The subclassing rationale is mainly to provide a valid reason for this ticket independent of other tickets. And it is good practice in general to move away from hard-coding values within method bodies, etc.

[Tim Graham]

For some reason, I thought you were proposing a class attribute, but now I see you wrote instance attribute. It may be a bit simpler to tackle #27008 straight away, as that's how the rest of the __init__() arguments were added, but I guess if you want to work in small chunks, the only disadvantage is more tickets and pull requests.

[Chris Jerdonek]

Pull request added ​here.

[Tim Graham <timograham@…>]

In a3a5ef4d:

Fixed #27035 -- Eased changing settings.DEBUG for DiscoverRunner.