Use mysqlclient-python #1126

giginet · 2017-02-11T14:45:23Z

To check CI status

lambdalisue · 2017-02-11T14:46:56Z

src/kawaz/settings.py

 from .pre_settings import *
 from django.utils.translation import ugettext_lazy as _

+IS_CI = os.environ['CI'] and os.environ['TRAVIS']


IS_TRAVIS で（Travis で DB のテーブル作ってるので Travis 依存になってるから）

lambdalisue · 2017-02-11T14:47:24Z

src/kawaz/settings.py

 except ImportError:
-    pass
+    if CI:
+        from .ci_settings import *


.travis_settings で（同様の理由）

.local_settings とは意味合い的に上下関係はないので別の try で

lambdalisue · 2017-02-11T14:48:21Z

src/kawaz/settings.py


 # django-debug-toolbar
 def show_debug_toolbar(request):
+    return False


これなんで？一時的なもの？

ミスった 😲

giginet · 2017-02-11T14:52:39Z

WIPになってるのは、まずTravis CIの設定をして、PyMySQLで落ちることを確認してからmysqlclientを入れたい

coveralls · 2017-02-11T14:59:47Z

Coverage decreased (-0.03%) to 97.648% when pulling ef55c54 on mysqlclient into bd7b78d on develop.

lambdalisue · 2017-02-11T16:12:11Z

src/lib/django-activities/activities/tests/test_models.py

        # the order of appearance is also important
        self.assertQuerysetEqual(activities,
-                                 (5, 4, 2, 1),
+                                 (activity5.pk, activity4.pk, activity2.pk, activity1.pk),


coveralls · 2017-03-26T12:10:41Z

Coverage decreased (-0.001%) to 97.695% when pulling 630c2b5 on mysqlclient into 3286efa on develop.

giginet · 2017-03-26T12:28:55Z

@lambdalisue I migrated DB for testing from sqlite to MySQL.
Could you review it?

coveralls · 2017-03-26T12:30:07Z

Coverage decreased (-0.03%) to 97.67% when pulling 4a4ceb1 on mysqlclient into 3286efa on develop.

coveralls · 2017-03-26T12:34:54Z

Coverage decreased (-0.001%) to 97.695% when pulling 4a4ceb1 on mysqlclient into 3286efa on develop.

lambdalisue

last() よりも first() のほうがベターなところが多々ある
順番保証テストが保持保証テストに変わってしまっているので見直す（順番保証が不要なら理由コメントほしい）
ordering で -pk に変更してる理由はわかるんだけど、1 とかで落ちてたから Django + MySQL の場合 pk の付け方ってシーケンシャルなのかな？という疑問。この辺の調査してくれると助かる。

lambdalisue · 2017-03-28T15:24:03Z

config/requirements-test.txt

 factory_boy>=2.7
 coverage
 faker
+mysqlclient


これは travis の事情（ローカルでは依然 sqlite を使う）なので travis.yml でインスコするべきかな

lambdalisue · 2017-03-28T15:25:05Z

src/kawaz/apps/announcements/tests/test_models.py

-        self.assertEqual(qs[0], b)
-        self.assertEqual(qs[1], a)
+        self.assertIn(b, qs)
+        self.assertIn(a, qs)


[question] ん、ここ出現順番のテスト崩してもいいの？出現順番を保証するようにするほうが妥当じゃない？

lambdalisue · 2017-03-28T15:31:31Z

src/kawaz/apps/announcements/tests/test_views.py

            'silently' : True
        })
-        self.assertRedirects(r, '/announcements/1/')
+        announcement = Announcement.objects.last()


全体的に last 使ってるけど first のほうがベターかな

self.assertEqual(Announcement.objects.count(), 1) が通ることからも分かる通り MySQL で落ちたのはデータが残っていたからではなく MySQL の pk の適用方法が異なるためだと思う（たぶんシーケンシャル pk じゃないんだと邪推）なので last でも first でも返ってくるものは同じだけど、仮にゴミデータが残っていた + count() をチェックしていないを満たすテストがあった場合

last を使った場合：最後に追加されたものが返るのでテストが通る

first を使った場合：最初に追加されたものが返るのでテストが落ちる

って違いが発生してミスに気が付きやすくなると思う。

lambdalisue · 2017-03-28T15:33:41Z

src/kawaz/apps/announcements/tests/test_views.py

            'silently' : True,
        })
-        self.assertRedirects(r, settings.LOGIN_URL + '?next=/announcements/1/update/')
+        self.assertRedirects(r, settings.LOGIN_URL + '?next=/announcements/{}/update/'.format(self.announcement.pk))


どのみち format() 使うなら '{}?next=...{}/update/'.format(settings.LOGIN...) にしたほうがいいかな

lambdalisue · 2017-03-28T15:34:38Z

src/kawaz/apps/announcements/tests/test_views.py

+        r = self.client.post('/announcements/{}/delete/'.format(self.announcement.pk), {})
        self.assertEqual(Announcement.objects.count(), 1)
-        self.assertRedirects(r, '{0}?next=/announcements/1/delete/'.format(settings.LOGIN_URL))
+        self.assertRedirects(r, '{0}?next=/announcements/{1}/delete/'.format(settings.LOGIN_URL, self.announcement.pk))


nil: 順番通り適用するなら {} を連続して使えば OK

lambdalisue · 2017-03-28T15:35:21Z

src/kawaz/apps/blogs/tests/test_models.py

-        self.assertEqual(qs[0], self.entries[1])
-        self.assertEqual(qs[1], self.entries[0])
+        self.assertIn(self.entries[0], qs)
+        self.assertIn(self.entries[1], qs)


ここも順番保証した方がいいと思う。MySQL で落ちるのであれば MySQL が返す順番がおかしいはず

lambdalisue · 2017-03-28T15:36:20Z

src/kawaz/apps/blogs/tests/views/test_crud.py

-        self.assertEqual(list[0], self.entries[1], 'protected')
-        self.assertEqual(list[1], self.entries[0], 'public')
+        self.assertIn(self.entries[1], list, 'protected')
+        self.assertIn(self.entries[0], list, 'public')


ここも順番保証のほうがいいかな

他にも何箇所かあったけど全部指摘するの面倒だから指摘してない。

lambdalisue · 2017-03-28T15:38:06Z

src/kawaz/apps/products/tests/test_views.py

                r = self.client.post(url, self.product_kwargs)
-            e = Product.objects.get(pk=i+1)
-            self.assertRedirects(r, e.get_absolute_url())
+            product = Product.objects.last()


ちなみにここは last() で正しい（あと i 使わないなら enumerate() も消したほうがいいかな）

lambdalisue · 2017-03-28T15:39:42Z

src/lib/django-activities/activities/models.py


    class Meta:
-        ordering = ('-created_at',)
+        ordering = ('-pk',)


[question]: MySQL での pk ってシーケンシャルなのかな？もしそうなら今まで直してきたテストはなんで落ちてたんだろうか（たぶんシーケンシャルだけど途中からとか？）

lambdalisue reviewed Feb 11, 2017

View reviewed changes

giginet force-pushed the mysqlclient branch from 8ddd534 to 64d34a4 Compare February 11, 2017 14:50

giginet force-pushed the mysqlclient branch from 64d34a4 to ef55c54 Compare February 11, 2017 14:55

giginet force-pushed the mysqlclient branch 4 times, most recently from 1b14fca to 7ac9bb4 Compare February 11, 2017 15:39

lambdalisue reviewed Feb 11, 2017

View reviewed changes

giginet mentioned this pull request Feb 11, 2017

Travis CIでMySQL使うようにする #1127

Open

giginet force-pushed the mysqlclient branch from b8de3b4 to 630c2b5 Compare March 26, 2017 12:02

giginet added 17 commits March 26, 2017 21:19

Remove unused setting

5de5b51

Use MySQL on Travis CI

e6c93bc

Use mysqlclient

0c8281c

Add default DB

0458e49

Add --noinput

7debc9d

Avoid hard cording of activity tests

5909405

Set order as pk

afaa3f4

Set storage_engine

1e91e33

Fix products test

537ad71

Improve tests of announcements

916f426

Improve tests of blog

3d04be3

Improve tests for events

cf4dc20

Improve tests for projects

be16e56

Fix variable names

ea6c52c

Fix test is failing

31830c3

Fix timezone issue on Travis CI

267cab9

Ignore order of test cases

68337ae

giginet force-pushed the mysqlclient branch from 630c2b5 to 68337ae Compare March 26, 2017 12:21

Remove unnecessay setting

4a4ceb1

giginet changed the title ~~[WIP] Use mysqlclient-python~~ Use mysqlclient-python Mar 26, 2017

lambdalisue suggested changes Mar 28, 2017

View reviewed changes

Use mysqlclient-python #1126

Are you sure you want to change the base?

Use mysqlclient-python #1126

Uh oh!

Conversation

giginet commented Feb 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

giginet commented Feb 11, 2017

Uh oh!

coveralls commented Feb 11, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Mar 26, 2017

Uh oh!

giginet commented Mar 26, 2017

Uh oh!

coveralls commented Mar 26, 2017

Uh oh!

coveralls commented Mar 26, 2017

Uh oh!

lambdalisue left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

giginet commented Feb 11, 2017 •

edited

Loading