Code

Opened 3 years ago

Closed 3 years ago

Last modified 17 months ago

#17055 closed Bug (fixed)

Fix the errors in the test suite when MySQL is the backend

Reported by: jsdalton Owned by: aaugustin
Component: Testing framework Version: master
Severity: Release blocker Keywords: mysql
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The work being done in #11665 had fixed all of the errors in the MySQL test run, but it's now broken again due to some recent movement in trunk. Due to other issues with that ticket, I decided it would be better if I split off the goal of fixing the MySQL errors into a separate ticket here.

I'll be attaching a patch that should fix all of the remaining errors in MySQL. The remaining issues are mostly an accumulation of minor errors that have accrued since MySQL does not get properly tested frequently enough. Here's a quick rundown of what the patch fixes:

  • There are tests in regressiontests.multiple_database that do some funky stuff with the data they load. Basically the load data that couldn't be loaded under normal circumstances. As of #3615 it's possible to disable constraint checks in MySQL to match the default testing behavior of the other backends, so I've done this in the pertinent spots. All tests now pass there.
  • This ticket: #16026 improved the error message when loaddata tries to load invalid data. However, MySQL throws a Warning instead of raising IntegrityError. I trapped the Warning and raise it as an exception and tweaked the test to account for that error type. Tests now pass in modeltests.fixtures.tests.FixtureLoadingTests.
  • regressiontests.admin_scripts.tests.ManageAlternateSettings was hitting some errors because MySQL uses backticks instead of quotation marks. The tests were assuming backends all used quotation marks. I changed some of the output tests to regex and now they pass.
  • django.contrib.auth.tests.tokens.TokenGeneratorTest.test_10265 says that a call to create_user and then a get of the same record should return the same password token. Because (at least for my MySQL installation) MySQL does not store microseconds, the second call to user.last_login (used to create the token) does not have microseconds. The only way to ensure these tokens are always identical is to normalize the last_login datetime so that it's stripped of microseconds. With this fix the test passes.

These are disparate issues however most of them are minor. They are, as it stands, the only issues preventing MySQL from passing all tests. I'm not very well versed in any of them in particular, but if anyone sees a problem with my fixes let me know.

Marking as release blocker to ensure these get fixed before 1.4.

Attachments (1)

fix_mysql_test_errors.diff (8.0 KB) - added by jsdalton 3 years ago.

Download all attachments as: .zip

Change History (11)

Changed 3 years ago by jsdalton

comment:1 Changed 3 years ago by jsdalton

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Whoops, just realized I included my fix for #17054 in this patch (reverting a test import in the i18n tests to a * import). That's not relevant to the MySQL test failures (though it was relevant to me because I couldn't run the test suite without it).

comment:2 Changed 3 years ago by aaugustin

  • Owner changed from nobody to aaugustin

Thanks for your work! We definitely need to fix the build on MySQL; there will be a higher incentive to keep it green afterwards. Re-assignig to myself to review the patch.

comment:3 Changed 3 years ago by aaugustin

In [16993]:

Fixed a test that was broken at r16978. Refs #17055.

comment:4 Changed 3 years ago by aaugustin

In [16994]:

Fixed a test that depended on how identifiers are quoted by the database backend and broke under MySQL. Refs #17055.

comment:5 Changed 3 years ago by aaugustin

I have committed two easy fixes. The other problems are more complicated:

  • we can't import MySQLdb in loaddata: it must be usable by people who don't have MySQL;
  • the test failures in multiple_database don't happen when tests are run in isolation. This is weird — django.test.TestCase should isolate tests properly. This deserves more investigation.
Last edited 3 years ago by aaugustin (previous) (diff)

comment:6 Changed 3 years ago by aaugustin

In [16995]:

Fixed multiple database tests that broke foreign keys constraints. Refs #17055.

The problem was masked by the rollback at the end of each transactional test on backends that deferred constraints checks; it appeared only on MySQL + InnoDB.

comment:7 Changed 3 years ago by aaugustin

In [16996]:

Fixed a test that relied on the database to reject invalid data; MySQL doesn't. Refs #17055.

comment:8 Changed 3 years ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to closed

As of http://ci.djangoproject.com/job/Django/381/ the build is green.

jsdalton, thank you for your analysis of the failing tests and for the initial patch.

comment:9 Changed 17 months ago by Aymeric Augustin <aymeric.augustin@…>

In 9bd67f056c6d1697d81841e3e21661069f57b14e:

[1.5.x] Fixed #16039 -- Made post_syncdb handlers multi-db aware.

Also reverted 8fb7a9002669fb7ba7bec7df90b465b92e1ed3c2. Refs #17055.

Backport of a026e48 from master.

comment:10 Changed 17 months ago by Aymeric Augustin <aymeric.augustin@…>

In a026e480da89cb99c9dc6c954fb5a37ded0f9315:

Fixed #16039 -- Made post_syncdb handlers multi-db aware.

Also reverted 8fb7a9002669fb7ba7bec7df90b465b92e1ed3c2. Refs #17055.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.