Bug#847555: tracker.debian.org: Functional testing suite seems broken
Raphael Hertzog <hertzog@debian.org> writes:
> On Fri, 09 Dec 2016, efkin wrote:
>> Raphael Hertzog <hertzog@debian.org> writes:
>>
>> > On Fri, 09 Dec 2016, efkin wrote:
>> >> I looked for the driver but it was not on stable/main so went for
>> >> chromedriver. And the situation improved considireously. Then realized
>> >> that improper method was used for closing the driver, leading to
>> >> undesired spwaning of driver processes.
>> >
>> > Thanks, I applied your patches. But while you say that the situation
>> > improved, I still see failures (both on jessie and on stretch/sid). Do you
>> > plan to look into those failures?
>>
>> I'm on it! Just attached some patches that should fix most part of
>> functional_test suite:
>
> Nice, a few comments to fix and I'll merge your work!
Thx! I'm attaching new patches in different order.
>> Subject: [PATCH 1/5] Update session hash after password change
>>
>> Since Django >= 1.7 SessionAuthenticationMiddleware invalidates
>> session after password change.
>>
>> def form_valid(self, form, *args, **kwargs):
>> form.save()
>> + update_session_auth_hash(self.request, form.user)
>> return super(PasswordChangeView, self).form_valid(form, *args, **kwargs)
>
> Do we want to revert this behaviour change of Django or should we rather
> fix the functional test to re-log after the password change?
> I did not look the reason for the change but I would rather be on the safe
> side and follow Django's default behaviour and hence modify the test.
You're right. It's a good concern. See patch #3 with the changes.
>> Subject: [PATCH 2/5] Add missing Architecture test fixture
>>
>> With this fixture, RepositoryAdminTest.test_respository_add()
>> can be executed smoothly.
>> ---
>> distro_tracker/core/fixtures/repository-architectures-test-fixture.json | 1 +
>> functional_tests/tests.py | 2 ++
>> 2 files changed, 3 insertions(+)
>> create mode 100644 distro_tracker/core/fixtures/repository-architectures-test-fixture.json
>
> Why do we need this fixture? The architectures should be created as part
> of the initial migrations, see
> distro_tracker/core/migrations/0002_initial_data.py and
> 0006_more_architectures.py.
>
>> + call_command('loaddata', 'distro_tracker/core/fixtures/repository-architectures-test-fixture.json', verbosity=0)
>
> If we really need a fixture, we should load it throught the fixture
> attribute on the class:
> https://docs.djangoproject.com/en/1.10/topics/testing/tools/#django.test.TransactionTestCase.fixtures
Well, i'm not sure if the migrations that you mentioned also works for
testing database. I tried to run test with verbose level 3 and the
migration is applied but no data is fulfilled. So i went for fixtures.
Next issue was that using fixture attribute worked only if the test was
executed singularly for the RepositoryAdminTestCase via `./manage.py
test functional_tests.tests.RepositoryAdminTestCase` but, if executed
the whole test suite it was not loading the fixture. Maybe due to
chaining of subclasses. So i used the `loaddata` command that is what
`fixtures` attribute calls under the hood i guess.
>> Subject: [PATCH 3/5] Fix wrong URL
>>
>> It was causing Timeout Error on test suite execution.
>> ---
>> # The user goes to the package page
>> - self.get_page('/' + package_name)
>> + self.get_page('/pkg/' + package_name)
>
> The "/foo" URL should end up being redirected to "/pkg/foo" if the package
> exists. Why is the redirection not properly working here?
It is working. It was my browser that was not loading fast enough the
redirect. So i excluded this patch from the new set.
>> From fba692e2e68e44745da137da10104fa1544016a9 Mon Sep 17 00:00:00 2001
>> From: efkin <efkin@riseup.net>
>> Date: Fri, 9 Dec 2016 19:07:28 +0100
>> Subject: [PATCH 5/5] Add missing html id tag
>>
>> It was violating SubscribeToPackageTest class.
>
> I think I dropped it in the conversion to bootstrap v4 because I found no
> use of it... I missed this obviously.
So i kept this patch on the patch set.
> Cheers,
Cheers,
> --
> Raphaël Hertzog ◈ Debian Developer
>
> Support Debian LTS: http://www.freexian.com/services/debian-lts.html
> Learn to master Debian: http://debian-handbook.info/get/
--
efkin.
>From e1c4f4879f7a99995e8c742aedd57e9e967cdeb2 Mon Sep 17 00:00:00 2001
From: efkin <efkin@riseup.net>
Date: Fri, 9 Dec 2016 17:41:17 +0100
Subject: [PATCH 1/4] Fix too short await time
This caused a Timeout Error on test suite execution.
---
functional_tests/tests.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/functional_tests/tests.py b/functional_tests/tests.py
index 37dbf0a..95ad084 100644
--- a/functional_tests/tests.py
+++ b/functional_tests/tests.py
@@ -790,7 +790,7 @@ class SubscribeToPackageTest(UserAccountsTestMixin, SeleniumTestCase):
# The subscribe button is no longer found in the page
button = self.get_element_by_id('subscribe-button')
# === Give the page a chance to refresh ===
- wait = ui.WebDriverWait(self.browser, 1)
+ wait = ui.WebDriverWait(self.browser, 2)
wait.until(lambda browser: not button.is_displayed())
self.assertFalse(button.is_displayed())
--
2.1.4
>From 6cb1c3ee6eb6626111c6cb8c7792bce88b5966f0 Mon Sep 17 00:00:00 2001
From: efkin <efkin@riseup.net>
Date: Fri, 9 Dec 2016 19:07:28 +0100
Subject: [PATCH 2/4] Add missing html id tag
It was violating SubscribeToPackageTest class.
---
distro_tracker/accounts/templates/accounts/subscriptions.html | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/distro_tracker/accounts/templates/accounts/subscriptions.html b/distro_tracker/accounts/templates/accounts/subscriptions.html
index ac52d67..0ce1c16 100644
--- a/distro_tracker/accounts/templates/accounts/subscriptions.html
+++ b/distro_tracker/accounts/templates/accounts/subscriptions.html
@@ -190,7 +190,7 @@
</div>
<div class="col-sm-9">
<input type="hidden" value="{% url 'dtracker-accounts-subscriptions' %}" name="next">
- <input class="form-control package-completion" type="text" name="package" placeholder="Choose package...">
+ <input id="package-search-input" class="form-control package-completion" type="text" name="package" placeholder="Choose package...">
</div>
</div>
<div class="form-group row">
--
2.1.4
>From c68b3f160fd7214b2ac0754d3c722489b002c5ba Mon Sep 17 00:00:00 2001
From: efkin <efkin@riseup.net>
Date: Sat, 10 Dec 2016 01:15:33 +0100
Subject: [PATCH 3/4] Log in back user after password change
Since Django >= 1.7 SessionAuthenticationMiddleware invalidates
session after password change.
---
functional_tests/tests.py | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/functional_tests/tests.py b/functional_tests/tests.py
index 95ad084..9585bbc 100644
--- a/functional_tests/tests.py
+++ b/functional_tests/tests.py
@@ -1089,13 +1089,17 @@ class ChangeProfileTest(UserAccountsTestMixin, SeleniumTestCase):
self.input_to_element('id_new_password1', new_password)
self.input_to_element('id_new_password2', new_password)
self.send_enter('id_new_password2')
+ old_password = self.password
+ self.password = new_password
+ self.log_in()
# The response shows a message confirming that the password
# was changed.
self.assert_in_page_body('Successfully updated your password')
-
+
# The user logs out in order to try using their new password.
self.click_link('Log out')
# The user tries logging in using their old account password.
+ self.password = old_password
self.log_in()
# The response shows an error message for incorrect credentials.
self.assert_in_page_body('Please enter a correct email and password')
--
2.1.4
>From d6769360c0c2a741129c16f8ce5179f7be726372 Mon Sep 17 00:00:00 2001
From: efkin <efkin@riseup.net>
Date: Fri, 9 Dec 2016 16:55:09 +0100
Subject: [PATCH 4/4] Add missing Architecture test fixture
With this fixture, RepositoryAdminTest.test_respository_add()
can be executed smoothly.
---
functional_tests/fixtures/repository-architectures-test-fixture.json | 1 +
functional_tests/tests.py | 2 ++
2 files changed, 3 insertions(+)
create mode 100644 functional_tests/fixtures/repository-architectures-test-fixture.json
diff --git a/functional_tests/fixtures/repository-architectures-test-fixture.json b/functional_tests/fixtures/repository-architectures-test-fixture.json
new file mode 100644
index 0000000..c2f9140
--- /dev/null
+++ b/functional_tests/fixtures/repository-architectures-test-fixture.json
@@ -0,0 +1 @@
+[{"fields": {"name": "amd64"}, "model": "core.architecture", "pk": 1}, {"fields": {"name": "armel"}, "model": "core.architecture", "pk": 2}, {"fields": {"name": "armhf"}, "model": "core.architecture", "pk": 3}, {"fields": {"name": "hurd-i386"}, "model": "core.architecture", "pk": 4}, {"fields": {"name": "i386"}, "model": "core.architecture", "pk": 5}, {"fields": {"name": "ia64"}, "model": "core.architecture", "pk": 6}, {"fields": {"name": "kfreebsd-amd64"}, "model": "core.architecture", "pk": 7}, {"fields": {"name": "kfreebsd-i386"}, "model": "core.architecture", "pk": 8}, {"fields": {"name": "mips"}, "model": "core.architecture", "pk": 9}, {"fields": {"name": "mipsel"}, "model": "core.architecture", "pk": 10}, {"fields": {"name": "powerpc"}, "model": "core.architecture", "pk": 11}, {"fields": {"name": "s390"}, "model": "core.architecture", "pk": 12}, {"fields": {"name": "s390x"}, "model": "core.architecture", "pk": 13}, {"fields": {"name": "sparc"}, "model": "core.architecture", "pk": 14}, {"fields": {"name": "all"}, "model": "core.architecture", "pk": 15}, {"fields": {"name": "any"}, "model": "core.architecture", "pk": 16}, {"fields": {"name": "alpha"}, "model": "core.architecture", "pk": 17}, {"fields": {"name": "arm"}, "model": "core.architecture", "pk": 18}, {"fields": {"name": "arm64"}, "model": "core.architecture", "pk": 19}, {"fields": {"name": "armeb"}, "model": "core.architecture", "pk": 20}, {"fields": {"name": "avr32"}, "model": "core.architecture", "pk": 21}, {"fields": {"name": "hppa"}, "model": "core.architecture", "pk": 22}, {"fields": {"name": "m32r"}, "model": "core.architecture", "pk": 23}, {"fields": {"name": "m68k"}, "model": "core.architecture", "pk": 24}, {"fields": {"name": "mips64"}, "model": "core.architecture", "pk": 25}, {"fields": {"name": "mips64el"}, "model": "core.architecture", "pk": 26}, {"fields": {"name": "mipsn32"}, "model": "core.architecture", "pk": 27}, {"fields": {"name": "mipsn32el"}, "model": "core.architecture", "pk": 28}, {"fields": {"name": "or1k"}, "model": "core.architecture", "pk": 29}, {"fields": {"name": "powerpcel"}, "model": "core.architecture", "pk": 30}, {"fields": {"name": "powerpcspe"}, "model": "core.architecture", "pk": 31}, {"fields": {"name": "ppc64"}, "model": "core.architecture", "pk": 32}, {"fields": {"name": "ppc64el"}, "model": "core.architecture", "pk": 33}, {"fields": {"name": "sh3"}, "model": "core.architecture", "pk": 34}, {"fields": {"name": "sh3eb"}, "model": "core.architecture", "pk": 35}, {"fields": {"name": "sh4"}, "model": "core.architecture", "pk": 36}, {"fields": {"name": "sh4eb"}, "model": "core.architecture", "pk": 37}, {"fields": {"name": "sparc64"}, "model": "core.architecture", "pk": 38}, {"fields": {"name": "x32"}, "model": "core.architecture", "pk": 39}]
\ No newline at end of file
diff --git a/functional_tests/tests.py b/functional_tests/tests.py
index 9585bbc..c05d5ab 100644
--- a/functional_tests/tests.py
+++ b/functional_tests/tests.py
@@ -13,6 +13,7 @@ Functional tests for Distro Tracker.
"""
from __future__ import unicode_literals
from distro_tracker.test import LiveServerTestCase
+from django.core.management import call_command
from django.core.urlresolvers import reverse
from django.contrib.auth import get_user_model
from django.core import mail
@@ -331,6 +332,7 @@ class RepositoryAdminTest(SeleniumTestCase):
main_email='admin',
password='admin'
)
+ call_command('loaddata', 'functional_tests/fixtures/repository-architectures-test-fixture.json', verbosity=0)
def login_to_admin(self, username='admin', password='admin'):
"""
--
2.1.4
Reply to: