Skip to content

Drop support for Django 1.7 #524

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
merged 2 commits into from
Oct 25, 2017
Merged

Drop support for Django 1.7 #524

merged 2 commits into from
Oct 25, 2017

Conversation

michael-k
Copy link
Contributor

As suggested in #521 (comment)

@michael-k michael-k mentioned this pull request Oct 9, 2017
@@ -1,4 +1,5 @@
# This file cannot be imported from until Django sets up
from django.db.backends.base.base import BaseDatabaseWrapper
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this (pytest_django.compat) a public API? If not, we could remove it and adjust the import in plugin.py.

@blueyed
Copy link
Contributor

blueyed commented Oct 10, 2017

Thanks for tackling this - the diff should be covered 100% though in this case I think.

@pytest-dev pytest-dev deleted a comment from codecov-io Oct 10, 2017
@pytest-dev pytest-dev deleted a comment from codecov-io Oct 10, 2017
@michael-k
Copy link
Contributor Author

the diff should be covered 100% though in this case I think.

I'll look into it this weekend or so.

@blueyed
Copy link
Contributor

blueyed commented Oct 18, 2017

Looking forward to it.
Let's wait for #521 to be merged first.

@blueyed
Copy link
Contributor

blueyed commented Oct 21, 2017

#521 is merged, so please rebase.

@michael-k
Copy link
Contributor Author

please rebase

done

@blueyed blueyed closed this Oct 23, 2017
@blueyed blueyed reopened this Oct 23, 2017
@pytest-dev pytest-dev deleted a comment from codecov-io Oct 23, 2017
@pytest-dev pytest-dev deleted a comment from codecov-io Oct 23, 2017
@blueyed blueyed closed this Oct 23, 2017
@blueyed blueyed reopened this Oct 23, 2017
@blueyed
Copy link
Contributor

blueyed commented Oct 23, 2017

close-reopen again for codecov to report back hopefully.

@codecov-io
Copy link

Codecov Report

Merging #524 into master will increase coverage by 0.2%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #524     +/-   ##
=========================================
+ Coverage   85.35%   85.55%   +0.2%     
=========================================
  Files          32       32             
  Lines        1625     1620      -5     
  Branches      141      140      -1     
=========================================
- Hits         1387     1386      -1     
+ Misses        182      178      -4     
  Partials       56       56
Flag Coverage Δ
#dj110 81.79% <50%> (+0.19%) ⬆️
#dj111 83.51% <50%> (+0.19%) ⬆️
#dj17 ?
#dj18 80.49% <50%> (+0.18%) ⬆️
#dj19 80.06% <50%> (+0.18%) ⬆️
#dj20 79.19% <50%> (+0.18%) ⬆️
#djmaster 79.19% <50%> (+0.18%) ⬆️
#mysql_innodb 79.07% <50%> (+0.18%) ⬆️
#mysql_myisam 79.01% <50%> (+0.18%) ⬆️
#postgres 81.54% <50%> (+0.18%) ⬆️
#py27 83.2% <50%> (+0.19%) ⬆️
#py34 81.11% <50%> (+0.18%) ⬆️
#py35 79.19% <50%> (+0.18%) ⬆️
#py36 81.23% <50%> (+0.18%) ⬆️
#pypy 80.37% <50%> (+0.18%) ⬆️
#pypy3 79.56% <50%> (+0.18%) ⬆️
#pytest3 85.55% <50%> (+0.2%) ⬆️
#sqlite_file 80.37% <50%> (+0.18%) ⬆️
Impacted Files Coverage Δ
tests/test_db_setup.py 100% <ø> (ø) ⬆️
tests/test_environment.py 100% <ø> (ø) ⬆️
tests/test_fixtures.py 100% <ø> (ø) ⬆️
pytest_django/plugin.py 67.37% <50%> (ø) ⬆️
pytest_django/fixtures.py 84.41% <50%> (+2.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1609780...1a8bfe2. Read the comment docs.

@codecov-io
Copy link

Codecov Report

Merging #524 into master will increase coverage by 0.2%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #524     +/-   ##
=========================================
+ Coverage   85.35%   85.55%   +0.2%     
=========================================
  Files          32       32             
  Lines        1625     1620      -5     
  Branches      141      140      -1     
=========================================
- Hits         1387     1386      -1     
+ Misses        182      178      -4     
  Partials       56       56
Flag Coverage Δ
#dj110 81.79% <50%> (+0.19%) ⬆️
#dj111 83.51% <50%> (+0.19%) ⬆️
#dj17 ?
#dj18 80.49% <50%> (+0.18%) ⬆️
#dj19 80.06% <50%> (+0.18%) ⬆️
#dj20 79.19% <50%> (+0.18%) ⬆️
#djmaster 79.19% <50%> (+0.18%) ⬆️
#mysql_innodb 79.07% <50%> (+0.18%) ⬆️
#mysql_myisam 79.01% <50%> (+0.18%) ⬆️
#postgres 81.54% <50%> (+0.18%) ⬆️
#py27 83.2% <50%> (+0.19%) ⬆️
#py34 81.11% <50%> (+0.18%) ⬆️
#py35 79.19% <50%> (+0.18%) ⬆️
#py36 81.23% <50%> (+0.18%) ⬆️
#pypy 80.37% <50%> (+0.18%) ⬆️
#pypy3 79.56% <50%> (+0.18%) ⬆️
#pytest3 85.55% <50%> (+0.2%) ⬆️
#sqlite_file 80.37% <50%> (+0.18%) ⬆️
Impacted Files Coverage Δ
tests/test_fixtures.py 100% <ø> (ø) ⬆️
tests/test_environment.py 100% <ø> (ø) ⬆️
tests/test_db_setup.py 100% <ø> (ø) ⬆️
pytest_django/plugin.py 67.37% <50%> (ø) ⬆️
pytest_django/fixtures.py 84.41% <50%> (+2.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1609780...1a8bfe2. Read the comment docs.

@codecov-io
Copy link

Codecov Report

Merging #524 into master will increase coverage by 0.2%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #524     +/-   ##
=========================================
+ Coverage   85.35%   85.55%   +0.2%     
=========================================
  Files          32       32             
  Lines        1625     1620      -5     
  Branches      141      140      -1     
=========================================
- Hits         1387     1386      -1     
+ Misses        182      178      -4     
  Partials       56       56
Flag Coverage Δ
#dj110 81.79% <50%> (+0.19%) ⬆️
#dj111 83.51% <50%> (+0.19%) ⬆️
#dj17 ?
#dj18 80.49% <50%> (+0.18%) ⬆️
#dj19 80.06% <50%> (+0.18%) ⬆️
#dj20 79.19% <50%> (+0.18%) ⬆️
#djmaster 79.19% <50%> (+0.18%) ⬆️
#mysql_innodb 79.07% <50%> (+0.18%) ⬆️
#mysql_myisam 79.01% <50%> (+0.18%) ⬆️
#postgres 81.54% <50%> (+0.18%) ⬆️
#py27 83.2% <50%> (+0.19%) ⬆️
#py34 81.11% <50%> (+0.18%) ⬆️
#py35 79.19% <50%> (+0.18%) ⬆️
#py36 81.23% <50%> (+0.18%) ⬆️
#pypy 80.37% <50%> (+0.18%) ⬆️
#pypy3 79.56% <50%> (+0.18%) ⬆️
#pytest3 85.55% <50%> (+0.2%) ⬆️
#sqlite_file 80.37% <50%> (+0.18%) ⬆️
Impacted Files Coverage Δ
tests/test_environment.py 100% <ø> (ø) ⬆️
tests/test_db_setup.py 100% <ø> (ø) ⬆️
tests/test_fixtures.py 100% <ø> (ø) ⬆️
pytest_django/plugin.py 67.37% <50%> (ø) ⬆️
pytest_django/fixtures.py 84.41% <50%> (+2.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1609780...1a8bfe2. Read the comment docs.

@blueyed
Copy link
Contributor

blueyed commented Oct 23, 2017

@michael-k
Copy link
Contributor Author

  • .

    # fixtures.py
    def django_db_setup(…):
        …
        if django_db_keepdb:
            if get_django_version() >= (1, 8):  # removed in this PR
                setup_databases_args['keepdb'] = True
            else:  # removed in this PR
                …  # removed in this PR
        …
    

    django_db_keepdb is True if request.config.getvalue('reuse_db') and …, but reuse_db/reuse-db is only set in test_db_setup.py. I do not know why this does not cover the above code.

  • The other thing that changed without coverage ist if get_django_version() >= (1, 8) and dj_settings.TEMPLATES: to if dj_settings.TEMPLATES: (2x) in plugin.py. def _fail_for_invalid_template_variable(request) is not covered at all and def _template_string_if_invalid_marker(request) has only one fully covered line. I have no idea how to cover the missing lines.

Conclusion:
The lines were not tested before. The changes (effectively replacing get_django_version() >= (1, 8) with True in conditions) don't affect the codebase at all for Django 1.8+. Diff coverage is missing because tests are missing, not because this PR made anything worse.
I do not regard the missing tests as an “easy pick”, so I'd say that they are out of scope of this PR. :(

@blueyed blueyed merged commit 218be96 into pytest-dev:master Oct 25, 2017
@blueyed
Copy link
Contributor

blueyed commented Oct 25, 2017

Thanks!

@michael-k michael-k deleted the drop-1.7 branch October 27, 2017 04:17
timb07 pushed a commit to timb07/pytest-django that referenced this pull request May 26, 2018

* Remove BaseDatabaseWrapper from compat
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