[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#847555: tracker.debian.org: Functional testing suite seems broken



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!

> 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 <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.

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/


Reply to: