microsoft / vscode-python

Python extension for Visual Studio Code
https://aka.ms/pvsc-marketplace
MIT License
4.29k stars 1.17k forks source link

Support user's test runner when executing Django unit tests #23961

Open swebra opened 4 weeks ago

swebra commented 4 weeks ago

Discovered while testing #23935 (#73's solution) in pre-release, the Django unit test support is partially implemented via the CustomExecutionTestRunner whose use is specified at test-time:

./manage.py --testrunner=django_test_runner.CustomExecutionTestRunner #...

This ignores any custom test runner the user may have set via the TEST_RUNNER Django setting, which may result in tests not being executable through the Test Explorer, or cause a difference in behaviour between executed tests manually and through the Test Explorer.

As linked above, Django supports user-defined test runners, and imo there are many use cases for them beyond supporting non-unittest testing frameworks. My particular use case is a test runner which overrides the setup_databases and teardown_databases methods to create and destroy unit test DBs with a Postgres admin user while still executing the tests with the default, lesser-permissioned DB user. This makes the testing more production-like as compared to the default Django experience. Without the custom test runner, the test execution fails as the default DB user (intentionally) does not have permission to create databases on its own.

Other simple uses cases may be adding additional CLI arguments to control the testing process, adding additional output to record attributes of the testing and its environment, or formatting the results for use in a secondary tool such as what xmlrunner/unittest-xml-reporting provides.

swebra commented 4 weeks ago

I guess it's worth adding that supporting non-conflicting, non-intersecting test runners would be a great start.

Looking at it now, the xmlrunner/unittest-xml-reporting example is probably the worst example I could have given; it is in direct conflict with CustomExecutionTestRunner in the sense that both are setting different values for kwargs["resultclass"]. Only supporting test runners which do not conflict with the basic customizations provided by CustomExecutionTestRunner would be a very reasonable limitation IMO. This would still allow user customization around input arguments, logging, system checks, database/environment setup/teardown, etc.

msmitherdc commented 3 weeks ago

We have a similar issue, we override the Test settings with a mixin

class TestSuiteRunner(SettingsTestMixin, DiscoverRunner):
    pass

and need to do the same with the VSCode runner.

msmitherdc commented 3 weeks ago

I was able to work around this by editing the django_test_runner.py and doing

class CustomExecutionTestRunner(SettingsTestMixin, BaseCustomExecutionTestRunner):
    pass

after changing VSCode's CustomExecutionTestRunner to BaseCustomExecutionTestRunner and adding our Mixin. Obviously not a workable solution going forward

msmitherdc commented 3 weeks ago

I've now been able to work around this by setting CustomExecutionTestRunner.setup_test_environment = setup_test_environment in our manage.py where setup_test_environment was the content of our Mixin

eleanorjboyd commented 3 weeks ago

Hello everyone! Thank you for this request and glad @msmitherdc you found a work-around! I attempted looking it up but as you both are likely more advanced with Django than myself is it possible to do multiple runners per run? I don't think so but wanted to check. You can take a look at the specific runner in the file mentioned, django_test_runner.py - which is called by the django_handler.py file. For discovery my custom runner needs to get access to the unittest.TestSuite and stop test run from happening (so only discovery runs but the tests dont) and then run I just need to resultclass to use my custom one UnittestTestResult so the tests get sent back after they run (and allows test statuses to be returned after each tests finishes and not just after the whole test run finishes). See it defined here.

@swebra with this in mind do you have a recommendation for how we could allow for both to be included? This is a similar problem to one I faced with the custom pytest runner but pytest allows multiple runners and so a line like this @pytest.hookimpl(hookwrapper=True, trylast=True) above a function to dictate how to works with other plugins in terms of ordering. Potentially maybe its to capture the user's custom runner in django_handler.py then somehow combine the users with the custom one we need? Not sure feasibility here and can investigate on my end too.

Thanks!

eleanorjboyd commented 3 weeks ago

Sorry one more follow up- does your custom plugin do anything on the discovery side? It seems like it doesn't really matter for collecting tests, that their name / location would be the same, and only impacts the database when run. If this is the case maybe we could just edit how run works to just change the resultclass. It doesn't seem like you can specify this without also specifying a custom runner so we would have to work around that by doing something like creating a custom arg and making sure it is parsed on either plugin. Opened to ideas on the best implementation here and what that could look like. Thanks

swebra commented 3 weeks ago

As far as I know, no, Django doesn't support multiple runners. The typical way to have the functionality of two test runners would be to combine the test runner classes through Python multi-inheritance like @msmitherdc also demonstrated above:

# in mysite/example_test_runner.py
# <required imports here>

class ExampleCombinedTestRunner(CustomTestRunner1, CustomTestRunner2):
   pass
# in mysite/settings.py
TEST_RUNNER = "mysite.example_test_runner.ExampleCombinedTestRunner"
# or alternatively passing the above string to the `--testrunner` arg

As per Python's MRO, this would prioritize methods that CustomTestRunner1 defines, followed by CustomTestRunner2's overrides, followed by their base classes (assumed to be DiscoverTestRunner).

With that in mind, I was imaging something along the lines of

  1. Dynamically importing the class defined by settings.TEST_RUNNER
  2. Performing any needed validation to give better error messages to the user than their tests not working
    • Does the custom test runner also override get_test_runner_kwargs? If so, error?
    • Does the custom test runner inherit from DiscoverRunner? If not, error?
  3. Having CustomExecutionRunner dynamically subclass settings.TEST_RUNNER, having a new runner which subclasses both, or some other way which calls settings.TEST_RUNNER's method definitions when needed. This would be the functional equivalent to
    class SuperCustomExecutionRunner(CustomExecutionRunner, <settings.TEST_RUNNER>):
       pass

For 1, see Django's getRunner function which is in turn called by the base test management command handler, though I do not have any specifics with regard to implementing 2 or 3.

My particular test runner does not do anything discovery-side, nor would any of the other presented examples (at least I think!), so perhaps working outside test runners would be possible... though imo it does sound a little hackier.

And just to be explicit though I think it's well understood, ideally the solution would eliminate/minimize any changes needed to a user's Django project (like to their test runner or settings). While the workarounds are great in the meantime, they do mean making vscode-specific code changes which have to be managed with non-VSCode developers. I think my proposal mostly avoids that, though I don't think entirely; An extension-level "Django test runner" setting might be desirable for example if your project has an unsupported test runner set by default and you can't/don't want to modify settings.py.

msmitherdc commented 3 weeks ago

@eleanorjboyd another change I've found from the VSCode runner from the default Django testrunner is the django runner https://docs.djangoproject.com/en/5.1/topics/testing/advanced/#django.test.utils.setup_test_environment in setting up the test environment changes the email default to a dummy email outbox. Something to note if the VSCode runner is going to initialize the test environment differently from django.

eleanorjboyd commented 3 weeks ago

@msmitherdc good catch! Let me think if there is a patch for this but I may also reach out to the Django repo as well to ask about the reasoning behind this / if they have advice. Thanks

msmitherdc commented 3 weeks ago

There is also an instrumented template render that is used with the Django runner. I was able to setup both mail and the template by doing in my test setup

if self.__class__.__name__ == 'CustomExecutionTestRunner': # eg vscode runner
    from django.test.utils import setup_test_environment
    setup_test_environment()