New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Plugin stops testing non-django projects #3
Comments
Importing mostly any part of Django requires I think the only way to solve this is to remove the entry_point, and require explicitly creating a conftest.py file with The entry_point might not be the best way to activate a py.test plugin like this. It might be a better fit for other, less intrusive and self-contained plugins (like xdist, coverage etc, which needs to be explicitly enable with command line flags). It might be possible to do some very nasty I have not encountered this myself for my non-Django projects since I have different virtualenvs, without pytest-django installed. What does your environment look like if you have pytest-django installed, but does want to test a project without Django? |
I install my normal working environment in the user site-packages (setup.py install --user). Would it be reasonable to keep the entrypoint but only enable the plugin if DJANGO_SETTINGS_MODULE is set? This way you'd still have failures due to missing db/funcargs if you didn't but the plugin won't be enabled until you have the environment variable set. |
Hmm, it would require some import tricks, but it would probably be possible and sounds like a good way to approach it. I am not sure if it would have other implications. It would be nice to get helpful errors if you try to use a funcarg/pytest.urls/pytest.fixtures which requires Django with settings, but does not have DJANGO_SETTINGS_MODULE set for instance. I don't have much time to work on it right now, but I would gladly review a pull request and merge such change if you want to put some effort into it! |
I've modified the plugin to do this, additionally it will use a DJANGO_SETTINGS_MODULE set in the ini file if present. In the process I ended up putting the entire plugin in a single module since that was easier and not too big (plus it's nice since you can easily make it available in a project without needing users to install it). I think most of the failures are fairly graceful (apart from when the test code needs DJANGO_SETTINGS_MODULE itself - don't think that can be helped), but haven't written any specific tests yet. All your tests apart from the live-server test pass as well, but as said it probably needs a few extra tests. Since I'll be away next week it will probably take a while before I polish it a bit more and can make a pull request. In the mean time I've created a gist so you can comment if you want: https://gist.github.com/2852134 |
Thanks for taking the time to straighten this out! I did not know about |
Thanks to both of you for tackling this. I also installed the plugin and was surprised to see "py.test -h" or "py.test --version" not work. Actually, i'd be happy with pytest-django requiring django>=1.3 and if tests requiring DJANGO_SETTINGS_MODULE (through use of funcargs) were skipped with an appropriate message. |
Holger, thanks for taking a look at the plugin. Any feedback from you on how to improve it would be very helpful! In the gist above, I think that the funcargs will still be registered and visible in |
I think it'd prefer skip - i might actually do such a thing on purpose aka "test all code that is not strictly django related). And skips are really there for situations where a dependency/platform does not match. Btw, i think a "--ds PATH" option would be nice - it would set DJANGE_SETTINGS_MODULE before the plugin tries importing Django. |
Good point on skipping instead of failing. And having --ds on top of DJANGO_SETTINGS_MODULE in the ini file seems reasonable too. I guess the priority is then (high to low): env var, command line, ini file. |
Agreed, it sounds reasonable to do skip instead of fail. In the original pytest-django code, there used to be a --settings option, which I removed, which configured DJANGO_SETTINGS_MODULE like you describe. I removed it because I think it is better to just configure Django the way Django is usually configured: setting DJANGO_SETTINGS_MODULE. Having set DJANGO_SETTINGS_MODULE gives other nice side effects, such as being able to run a python shell directly and import modules which depends on Django without having to go through other hoops. This also applies to any other Python tools, not just py.test, which can just import the Django modules without any additional configuration, just as with any other Python modules. I think this makes the compatibility with Django and standard Python tools better. There is also a FAQ which describes how to do it to avoid the typing when invoking py.test for Django tests: http://pytest-django.readthedocs.org/en/latest/faq.html#how-can-i-avoid-having-to-type-django-settings-module-to-run-the-tests Are there any advantages of writing |
When you say ini-file, you mean a django ini-file, right? Regarding the command line option: Does the "name=var command" syntax work on windows? Personally, i prefer being able to do without env-settings but i also see the tradional aspect of going with requiring DJANGO_SETTINGS_MODULE. In the end, i could use "tox" to do the env-settings for me and/or pytest grows a generic way to set environment variables in the [pytest] section. What about my other question, pytest-django specifying in its pypi-upload to require djange>=1.3? |
I was talking about pytest.ini, my version of the plugin reads DJANGO_SETTINGS_MODULE from the [pytest] section and sets it in os.environ if not already present. I have no strong opinion on having a command line option override but do feel strongly about being able to set it in pytest.ini somehow (or setup.cfg, tox.ini). If this is via a generic way of setting environment variables in the [pytest] section that would be fine too. As for unanswered questions: Andreas what is your opinion of me putting it all in one file? Is this acceptable or would you like to keep the package layout as is? I can probably get that to work too, though maybe a little messier. |
Regarding --ds: I am not sure about the environment variable syntax on windows. Yeah, I think we should accomodate for different use cases/work flows, and if the --ds option would help with that I will add it. A way to specify environment variables directly in pytest.ini would indeed be nice, but until then this option will be good enough. I opened a new issue to track it: #4 Regarding django>=1.3 in pypi required: Sorry, I missed that question. Do you think it should be added to the depencies so that it is automatically installed with pytest-django? I guess py.test should be added then too. I opened a new issue to track it: #5 I will try to find some time to fix them tonight. @flub Unless it presents issues with solving this issue, I would like the modules to stay. Since your initial gist I have added another module: pytest_django.db_reuse, and I like to keep the code separated into modules. Maybe a pytest_django.lazy_django module could be added which provides functions for lazily importing Django? |
@pelme thanks for the new issues - all makes sense to me. looking forward to play with the new plugin :) (Actually i am considering writing a little web service with django - would be my first django project :) |
Awesome! :) py.test really makes testing python code so much nicer. As always, let me know if anything can be improved! |
Another thing I did in my gist was abolish globals, instead storing the suite_runner etc on the pytest session since I think that is where they logically belong. Any objections against that or should I do that as a separate issue/patch? |
@flub It sounds reasonable to me and I think it is fine to keep it in the same patch. |
I just tried to test this in pytest-django's own test suite: e1f60494 This can not be tested in the standard pytest-django test suite, since Django will already be loaded in it. My idea is to invoke @hpk42 I added a new run_all_tests.sh script, which does the above, and invoke it from tox, but tox (rightfully) complains because that script is outside of the testenv. Do you have any ideas on how to test this in a good way? |
You can write tests using the "testdir" argument and easily invoke a https://bitbucket.org/hpk42/pytest/src/eabc28a991bb/testing/acceptance_test.py#cl-413 On Thu, Jun 14, 2012 at 11:12 PM, Andreas Pelme
|
I have ported the changes from your gist above with some modifications. I have moved all Django imports into the functions instead of using py.std.django, I think it becomes more explicit (and pyflakes is not very useful with py.std), and there is no need for the do_django_imports. The tests are now skipped instead of failed, and I added proper tests as per Holgers feedback. I haven't merged this to the master branch yet, and would love to hear your feedback first. The branch is here: pip install copy-paste: The tests for non-django-test runs are here: |
It looks good! Two nits:
But we discussed this elsewhere already IIRC. best, On Sun, Jun 17, 2012 at 9:24 PM, Andreas Pelme
|
One thing: the result.stdout.fnmatch_lines([...]) can take multiple On Sun, Jun 17, 2012 at 9:53 PM, Holger Krekel holger.krekel@gmail.com wrote:
|
Thanks for the quick feedback! I tried fnmatch_lines with multiple lines before, but I had the order of the lines mixed up then, so I didn't get it to pass, it is however fixed now. The setup.py issue is now fixed too. I will soon fix the pytest.urls stuff along with some other mark fixes (the transaction_test_case mark is not properly registered with pytest right now). |
Oh wow, I'm clearly too slow again. Just too much going on! Thanks for moving this along though. Everything seems to be fine, importing inside functions is a viable alternative. I only have a few observations:
Which is mostly a copy of The code does work fine in my environment so can be merged to master I would think. Thanks again for beating me to writing this. |
Did you see pytest_django.lazy_django.django_is_usable: https://github.com/pelme/pytest_django/blob/lazy_django/pytest_django/lazy_django.py ? Will that be enough? It is only usable after I will fix the funcarg docstrings too, and then push a new release to pypi! |
As you point out, only works if the plugin was loaded and I would like this at pytest_configure() time. Calling |
Right, I can introduce a new function which just returns the django settings module, which does not modify os.environ. It can then be used like this when the environment is setup:
Then you can use it from your code wherever you like and make sure it doesn't give nasty side effects, without having to duplicate the code. |
Sounds good, one request tough: only require a config object rather then a session object. I don't think you need the full session to find the django settings module and I would like to call this before a session is created. |
If there are multiple bits and helpers made available on the pytest best, On Mon, Jun 18, 2012 at 12:06 PM, Floris Bruynooghe
|
@flub It sounds reasonable to pass the config object instead, I am not very familiar with the session/config structure yet. @hpk42 Currently nothing is added to the pytest namespace except for urls -- which should soon be gone too. Would it be preferred to use |
Yes, using the pytest namespace seems good for this. But this made me think it would be even better to move the finding of the settings module to the pytest_configure() hook. This is called before my conftest.py:pytest_configure so I can use the result simply by config.getvalue('ds') and even use django_is_usable() if I want. So in plugin.py I now have:
And I think this would be the best approach. |
Yes, i recommend using pytest.django as the namespace for offering On Mon, Jun 18, 2012 at 2:10 PM, Andreas Pelme
|
I will update the code with your comments and document the new ways of specifiying DJANGO_SETTINGS_MODULE and push out a new version to PyPi. Thanks a lot for the feedback! |
This is now fixed! 🍰 |
Doesn't look like this is fixed for all cases, as
|
@ask Are you sure you are not accidentally importing from django somewhere during test collection? We do check that you actually imported Django and configured Django before calling pytest-django/pytest_django/plugin.py Lines 137 to 148 in e540ace
I did a quick test but was not able to reproduce the problem:
Please get back with more info on how to reproduce and I will look into it! |
Hi
When the plugin is installed testing a non-django project without having DJANGO_SETTINGS_MODULE results in an error. The reason (AFAIK) is that pytest_django.plugin, which is registered as the pytest11 entrypoint via "from .plugin import *" in pytest_django.init, does a direct "from django.conf import settings".
I'm not immediately sure how the plugin can detect it is testing a django application in order to decide to activate itself or not. But I think it would be beneficial to not have pytest-django break normal test runs by default.
Regards,
Floris
The text was updated successfully, but these errors were encountered: