Multidb testing for travis #1513

Merged
merged 29 commits into from Dec 20, 2012

Projects

None yet

6 participants

@digi604
Member
digi604 commented Nov 14, 2012

testing the db testing here and hopefully fixing the broken tests

@digi604
Member
digi604 commented Nov 14, 2012

mysql: (failures=26, errors=103, skipped=1)
postgres: (failures=7, errors=4)

@digi604
Member
digi604 commented Nov 14, 2012

Referencing some tickets: #1032, #1477

@ojii
Collaborator
ojii commented Nov 29, 2012

As mentioned on #1477, is there a way to signal this through a single command line arg/env variable (DATABASE_URL?)?

On travis we should be able to just change the script to DATABSE_URL="$DB://root@localhost/djangocms_test" python runtests.py

Would make it a lot easier to run locally instead of having to pass (up to) 4 arguments to the script.

@ojii
Collaborator
ojii commented Dec 17, 2012

Eg postgres://username:password@host:port/dbname. See dj_database_url on
github
On Dec 17, 2012 1:07 PM, "Patrick Lauber" notifications@github.com wrote:

How would you encode the database name in the url?


Reply to this email directly or view it on GitHubhttps://github.com/divio/django-cms/pull/1513#issuecomment-11439323.

@ojii ojii and 1 other commented on an outdated diff Dec 18, 2012
cms/tests/menu.py
def test_show_menu_num_queries(self):
context = self.get_context()
# test standard show_menu
- with self.assertNumQueries(5):
- """
- The queries should be:
- get all pages
- get all page permissions
- get all titles
- get the menu cache key
- set the menu cache key
- """
- tpl = Template("{% load menu_tags %}{% show_menu %}")
- tpl.render(context)
+ if settings.DATABASES['default']['ENGINE'] == 'django.db.backends.sqlite':
@ojii
ojii Dec 18, 2012 Collaborator

why this check?!

@digi604
digi604 Dec 18, 2012 Member

mysql innodb and postgres have transaction queries.

@ojii
ojii Dec 18, 2012 Collaborator

isn't this a Django bug then? Is this tracked somewhere?

Also since this test only does assertNumQueries (according to the name), why not use a conditional unittest.skip decorator?

@digi604
digi604 Dec 18, 2012 Member

which would you use?

@digi604
digi604 Dec 19, 2012 Member

@skipIfDBFeature('supports_transactions') does not work as it will skip for sqlite as well.

@ojii ojii and 1 other commented on an outdated diff Dec 18, 2012
cms/test_utils/cli.py
gettext = lambda s: s
urlpatterns = []
DJANGO_1_3 = LooseVersion(django.get_version()) < LooseVersion('1.4')
-def configure(**extra):
+def configure(db_url, **extra):
+ splits = db_url.split("://")
@ojii
ojii Dec 18, 2012 Collaborator

why not use dj_database_url here? This looks just insane, no offense. At the very least use urlparse properly here.

@digi604
digi604 Dec 18, 2012 Member

python 2.5 urlparse does not like custom url schemes... needed to do that this way

@ojii
ojii Dec 18, 2012 Collaborator
jonas:~ python2.5
Python 2.5.6 (r256:88840, Dec 16 2011, 14:22:12) 
[GCC 4.6.1] on linux3
Type "help", "copyright", "credits" or "license" for more information.
>>> from dj_database_url import parse
>>> parse('postgres://user:password@host:1191/dbname')
{'ENGINE': 'django.db.backends.postgresql_psycopg2', 'NAME': 'dbname', 'HOST': 'host', 'USER': 'user', 'PASSWORD': 'password', 'PORT': 1191}

where's the problem?

@GotenXiao GotenXiao commented on an outdated diff Dec 18, 2012
cms/models/pagemodel.py
@@ -860,15 +863,19 @@ def is_home(self):
def get_home_pk_cache(self):
attr = "%s_home_pk_cache_%s" % (self.publisher_is_draft and "draft" or "public", self.site_id)
- if not hasattr(self, attr):
+ if not hasattr(self, attr) or getattr(self, attr) is None:
@GotenXiao
GotenXiao Dec 18, 2012 Contributor

This could probably be simplified out to:

if getattr(self, attr, None) is None:
@adaptivelogic adaptivelogic commented on an outdated diff Dec 18, 2012
cms/test_utils/cli.py
+ },
+ }
+ if db_splits.username:
+ DB['USER'] = db_splits.username
+ if db_splits.password:
+ DB['PASSWORD'] = db_splits.password
+ if db_splits.hostname:
+ DB['HOST'] = db_splits.hostname
+ try:
+ if db_splits.port:
+ DB['PORT'] = db_splits.port
+ except ValueError:
+ pass
+ else:
+ DB = {}
+ print DB
@adaptivelogic
adaptivelogic Dec 18, 2012 Contributor

No prints in the test code, please

@digi604
Member
digi604 commented Dec 18, 2012

please check again

@digi604 digi604 merged commit 9c70a95 into divio:develop Dec 20, 2012

1 check passed

default The Travis build passed
Details
@digi604 digi604 deleted the digi604:multidb_test branch Dec 20, 2012
@coveralls

Coverage Status

Changes Unknown when pulling 113e386 on digi604:multidb_test into * on divio:develop*.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment