django-cms / djangocms-versioning

General purpose versioning package for Django CMS 4 and above.
Other
33 stars 29 forks source link

feat: Django 4.0, 4.1 / Python 3.10/3.11, mysql support, running tests on sqlite, postgres and mysql #287

Closed fsbraun closed 1 year ago

fsbraun commented 2 years ago

Description

This PR extends the test suite considerably by introducing tests for Postgres and MySQL. It is a fix for #286 where a migration that fails to run on MySQL.

It turns out that some unit tests did neither run on Postgres nor MySQL, since they compared pk of Version objects with pk of the corresponding Content object which only accidentally worked on sqlite.

Features of the PR

Checklist

codecov[bot] commented 2 years ago

Codecov Report

Merging #287 (f82d312) into master (31f14df) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #287   +/-   ##
=======================================
  Coverage   86.60%   86.60%           
=======================================
  Files          23       23           
  Lines         799      799           
  Branches      105      117   +12     
=======================================
  Hits          692      692           
  Misses         81       81           
  Partials       26       26           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Aiky30 commented 2 years ago

@fsbraun Can you add more details to the error / issue such as MySQL version and the database backend in use, for postgres this tends to be: django.db.backends.postgresql, although this is up to the project author: https://docs.djangoproject.com/en/4.1/ref/databases/#postgresql-connection-settings

I ask because this chnage doesn't single out MySQL and affects all DB's. Ideally we would try and avoid that.

fsbraun commented 2 years ago

I've added test for the mysql backend: django.db.backends.mysql and django.db.backends.postgresql. The fix now only affects the mysql backend. The tests run the migrations (that's why I added migrations for the test apps, too).

At this point it turned out that some tests were confusing Version object pk with Content object pk. I've fixed that.