Skip to content
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

Test failures after upgrading Django app to mysqlclient 1.3.14 #303

Closed
edmorley opened this issue Dec 6, 2018 · 2 comments · Fixed by #327
Closed

Test failures after upgrading Django app to mysqlclient 1.3.14 #303

edmorley opened this issue Dec 6, 2018 · 2 comments · Fixed by #327
Labels

Comments

@edmorley
Copy link

edmorley commented Dec 6, 2018

Hi

STR:

  1. Have a Django 1.11 app whose tests make use of CaptureQueriesContext, such as:
  2. Upgrade the mysqlclient used by that app from 1.3.13 to 1.3.14, eg:
  3. Run test suite (either locally or on Travis)

Expected:

No test failures after upgrading, since:

  • it was a minor release
  • there was no mention of removing _last_executed in the changelog

Actual:

Test failures, eg:

        with CaptureQueriesContext(connection) as context:
            Bugscache.search(search_term)
    
            search_query = get_search_query(context.captured_queries[0]['sql'])
>           assert search_query
E           AssertionError: assert ''

(https://travis-ci.org/mozilla/treeherder/jobs/463284994#L605)

Whilst Django's handling has been updated for Django 2.1+, this leaves Django 1.11 (LTS) and Django 2.0 incompatible with latest mysqlclient.

If restoring _last_executed ends up being wontfix, please can the changelog be updated to mention the removal (and if possible a reference to say Django <2.1 users should not update)?

Many thanks :-)

Refs:

@methane
Copy link
Member

methane commented Dec 6, 2018

Expected:

No test failures after upgrading, since:

  • it was a minor release
  • there was no mention of removing _last_executed in the changelog

When using private member, both are expected.

@methane methane closed this as completed in 509b6ce Dec 6, 2018
@edmorley
Copy link
Author

edmorley commented Dec 6, 2018

When using private member, both are expected.

Yeah is fair :-) Thank you for the changelog update.

@methane methane reopened this Jan 30, 2019
@methane methane added the Django label Jan 30, 2019
methane added a commit that referenced this issue Feb 7, 2019
Django touched private area of this library.
Removing unused variables broke Django.

While Django 2.0 fixed it, Django 1.11 doesn't fix it
because it is in security-only fix mode.

So add unused variables for Django 1.11 compatibility.
They will be removed in next minor release.

Fix #303, #306
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants