Skip to content

Make tests fail on warnings #1197

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

Merged

Conversation

adamchainz
Copy link
Contributor

Problem

Users seeing warnings.

Solution

This is a technique I've rolled out against my own open source projects and Django REST Framework.

By failing tests as soon as new deprecation warnings are created in the library, they aren't passed on to users who then have to report them back in issues like #1195, saving everyone some time, effort, and noise.

Acceptance Criteria

There are no warnings at current so the tests don't fail. To check I reverted the code changes from commit e855fcf, ran Django 3.1 tests, and saw the warnings fail the test run at import time:

GLOB sdist-make: /.../django-import-export/setup.py
py38-django31-tablibstable inst-nodeps: /.../django-import-export/.tox/.tmp/package/1/django-import-export-2.4.1.dev0.zip
py38-django31-tablibstable installed: asgiref==3.2.10,certifi==2020.6.20,chardet==3.0.4,coverage==5.3,coveralls==2.1.2,defusedxml==0.6.0,diff-match-patch==20200713,Django==3.1.2,django-import-export @ file:///.../django-import-export/.tox/.tmp/package/1/django-import-export-2.4.1.dev0.zip,docopt==0.6.2,et-xmlfile==1.0.1,idna==2.10,isort==5.6.4,jdcal==1.4.1,MarkupPy==1.14,mysqlclient==2.0.1,odfpy==1.4.1,openpyxl==3.0.5,psycopg2==2.8.6,pytz==2020.1,PyYAML==5.3.1,requests==2.24.0,sqlparse==0.4.1,tablib==2.0.0,urllib3==1.25.10,xlrd==1.2.0,xlwt==1.3.0
py38-django31-tablibstable run-test-pre: PYTHONHASHSEED='903442151'
py38-django31-tablibstable run-test: commands[0] | python -W error::DeprecationWarning -W error::PendingDeprecationWarning /.../django-import-export/tests/manage.py test core
Traceback (most recent call last):
  File "tests/manage.py", line 12, in <module>
    execute_from_command_line(sys.argv)
  File "/.../django-import-export/.tox/py38-django31-tablibstable/lib/python3.8/site-packages/django/core/management/__init__.py", line 401, in execute_from_command_line
    utility.execute()
  File "/.../django-import-export/.tox/py38-django31-tablibstable/lib/python3.8/site-packages/django/core/management/__init__.py", line 377, in execute
    django.setup()
  File "/.../django-import-export/.tox/py38-django31-tablibstable/lib/python3.8/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/.../django-import-export/.tox/py38-django31-tablibstable/lib/python3.8/site-packages/django/apps/registry.py", line 122, in populate
    app_config.ready()
  File "/.../django-import-export/.tox/py38-django31-tablibstable/lib/python3.8/site-packages/django/contrib/admin/apps.py", line 24, in ready
    self.module.autodiscover()
  File "/.../django-import-export/.tox/py38-django31-tablibstable/lib/python3.8/site-packages/django/contrib/admin/__init__.py", line 24, in autodiscover
    autodiscover_modules('admin', register_to=site)
  File "/.../django-import-export/.tox/py38-django31-tablibstable/lib/python3.8/site-packages/django/utils/module_loading.py", line 47, in autodiscover_modules
    import_module('%s.%s' % (app_config.name, module_to_search))
  File "/Users/chainz/.pyenv/versions/3.8.5/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "tests/../import_export/admin.py", line 25, in <module>
    from .signals import post_export, post_import
  File "tests/../import_export/signals.py", line 3, in <module>
    post_export = Signal(providing_args=["model"])
  File "/.../django-import-export/.tox/py38-django31-tablibstable/lib/python3.8/site-packages/django/dispatch/dispatcher.py", line 36, in __init__
    warnings.warn(
django.utils.deprecation.RemovedInDjango40Warning: The providing_args argument is deprecated. As it is purely documentational, it has no replacement. If you rely on this argument as documentation, you can move the text to a code comment or docstring.
ERROR: InvocationError for command /.../django-import-export/.tox/py38-django31-tablibstable/bin/python -W error::DeprecationWarning -W error::PendingDeprecationWarning tests/manage.py test core (exited with code 1)
___________________________________________________________________________________________________ summary ____________________________________________________________________________________________________
ERROR:   py38-django31-tablibstable: commands failed

This is a technique I've rolled out against my own open source projects and [Django REST Framework](encode/django-rest-framework#7586).

By failing tests as soon as new deprecation warnings are created in the library, they aren't passed on to users who then have to report them back in issues like django-import-export#1195, saving everyone some time, effort, and noise.
@coveralls
Copy link

coveralls commented Oct 16, 2020

Coverage Status

Coverage remained the same at 96.74% when pulling a5b59c7 on adamchainz:issue_1195_warnings into 691baad on django-import-export:master.

Copy link
Member

@andrewgy8 andrewgy8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Itll be interesting to see how this effects future development. Thanks!

@andrewgy8 andrewgy8 merged commit 3225f8a into django-import-export:master Oct 19, 2020
@adamchainz adamchainz deleted the issue_1195_warnings branch October 19, 2020 08:48
adamchainz added a commit to adamchainz/django-import-export that referenced this pull request Oct 19, 2020
Missed in django-import-export#1197, which changed only the tox configuration and not for Travis.
adamchainz added a commit to adamchainz/django-import-export that referenced this pull request Oct 19, 2020
Missed in django-import-export#1197, which changed only the tox configuration and not for Travis.
ZuluPro pushed a commit to ZuluPro/django-import-export that referenced this pull request Dec 23, 2020
This is a technique I've rolled out against my own open source projects and [Django REST Framework](encode/django-rest-framework#7586).

By failing tests as soon as new deprecation warnings are created in the library, they aren't passed on to users who then have to report them back in issues like django-import-export#1195, saving everyone some time, effort, and noise.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants