On Fri, 09 Dec 2016, efkin wrote:
> Raphael Hertzog <hert...@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!
> 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.
> 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
> 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?
> From fba692e2e68e44745da137da10104fa1544016a9 Mon Sep 17 00:00:00 2001
> From: efkin <ef...@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.
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/
