Project

General

Profile

Development #41250

Instabilité des tests multitenant en python3

Added by Paul Marillonnet 2 months ago. Updated about 2 months ago.

Status:
Solution déployée
Priority:
Normal
Category:
-
Target version:
-
Start date:
02 Apr 2020
Due date:
% Done:

0%

Patch proposed:
Yes
Planning:
No

Description

Lancés avec tox par Jenkins, en environnement py3-coverage-multitenant :

tests_multitenant/test_request_context_filter.py .F                      [ 34%]
[…]
    def test_systemd(settings, tenants, client, journald_handler):
        root_logger = logging.getLogger()
        assert len(root_logger.handlers) == 2
        journald_handler.addFilter(RequestContextFilter())

        for tenant in tenants:
            with tenant_context(tenant):
                user = User.objects.create(first_name='John', last_name='Doe', username='john.doe',
                                           email='jodn.doe@example.com')
                user.set_password('john.doe')
                user.save()
                user.saml_identifiers.create(name_id='ab' * 16, issuer='https://idp.example.com')

        for tenant in tenants:
            settings.ALLOWED_HOSTS.append(tenant.domain_url)
            with tenant_context(tenant):
                client.login(username='john.doe', password='john.doe')
            client.get('/', SERVER_NAME=tenant.domain_url,
                       HTTP_X_FORWARDED_FOR='99.99.99.99, 127.0.0.1')

        from systemd.journal import Reader
        import time

        reader = Reader()
        reader.seek_realtime(time.time() - 10)
        records = [l for l in reader if l['MESSAGE'] == 'wat!']
>       assert len(records) == 2
E       AssertionError: assert 4 == 2
E        +  where 4 = len([{'APPLICATION': 'fake-agent', 'ARGS': '()', 'CODE_FILE': '/var/lib/jenkins/workspace/hobo-wip_wip_41237-tox-a2-py3/ho... 'ARGS': '()', 'CODE_FILE': '/var/lib/jenkins/workspace/hobo/hobo/agent/test_urls.py', 'CODE_FUNC': 'helloworld', ...}])

tests_multitenant/test_request_context_filter.py:84: AssertionError

C'est ici par exemple.

On se retrouve avec un 'CODE_FILE': '/var/lib/jenkins/workspace/hobo-wip_wip_41237-tox-a2-py3/…' comme s'il n'y avait pas d'isolation entre les jobs hobo et hobo-wip en cas d'exécution simultanée.

0001-tests_multitenant-mock-journald-sender-41250.patch View (2.64 KB) Benjamin Dauvergne, 13 Apr 2020 11:47 AM

0001-tests_multitenant-mock-journald-sender-41250.patch View (3.59 KB) Benjamin Dauvergne, 13 Apr 2020 02:18 PM

Associated revisions

Revision 2b0474a2 (diff)
Added by Benjamin Dauvergne about 2 months ago

tests_multitenant: mock journald sender (#41250)

History

#1 Updated by Thomas Noël 2 months ago

Effectivement c'est un test particulier qui joue avec hobo.journal.JournalHandler, il faut certainement creuser ici pour tenter de voir comment isoler cela, car il y a certainement possibilité de mélange quand deux tests tournent.

#2 Updated by Benjamin Dauvergne about 2 months ago

  • Assignee set to Benjamin Dauvergne

#3 Updated by Benjamin Dauvergne about 2 months ago

  • Patch proposed changed from No to Yes
  • File 0001-tests-make-provisionning-test-less-instable-41250.patch added
  • Tracker changed from Bug to Development
  • Status changed from Nouveau to Solution proposée

#4 Updated by Thomas Noël about 2 months ago

C'est vraiment en rapport avec l'erreur journald ?

#5 Updated by Benjamin Dauvergne about 2 months ago

Non tu as raison; ça correspond à un souci causé par un changement dans authentic corrigé dans #41284 (on avait une écriture du compte au login qui n'existait pas avant, ça changeait le nombre d'appels à notify_agents), oublions, j'ai mélangé.

#6 Updated by Benjamin Dauvergne about 2 months ago

  • File deleted (0001-tests-make-provisionning-test-less-instable-41250.patch)

#9 Updated by Paul Marillonnet about 2 months ago

  • Status changed from Solution proposée to Solution validée

Ça m'embête un peu que dans ce test systemd on retire l'import à… systemd.
Je verrais bien du mocking un peu plus en profondeur pour ne pas virer cet import qui à mon avis donne du sens au test, mais je n'ai pas le temps de le faire (et peut-être pas les capacités non plus ☺).
Si tu préfères laisser comme ça, je comprends, pousse donc.

#10 Updated by Benjamin Dauvergne about 2 months ago

Paul Marillonnet a écrit :

Ça m'embête un peu que dans ce test systemd on retire l'import à… systemd.
Je verrais bien du mocking un peu plus en profondeur pour ne pas virer cet import qui à mon avis donne du sens au test, mais je n'ai pas le temps de le faire (et peut-être pas les capacités non plus ☺).
Si tu préfères laisser comme ça, je comprends, pousse donc.

Je ne comprends pas bien, le test utilise hobo.journal.JournalHandler qui importe justement systemd, tu préfèrerais un mock de systemd.journal.send ?

PS: ça n'est pas vraiment possible, systemd.journal.send est une valeur par défaut d'un argument du constructeur de JournalHandler, je ne peux pas le mocker, la valeur mockée ne sera pas prise en compte par le constructeur car elle est copié par valeur dans la définition de JournalHandler.__init__.

#11 Updated by Benjamin Dauvergne about 2 months ago

  • Status changed from Solution validée to Résolu (à déployer)
commit 2b0474a2e91b04a622587207dfee3c15edcc58ce
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Mon Apr 13 11:47:31 2020 +0200

    tests_multitenant: mock journald sender (#41250)

#12 Updated by Paul Marillonnet about 2 months ago

Benjamin Dauvergne a écrit :

Je ne comprends pas bien, le test utilise hobo.journal.JournalHandler qui importe justement systemd, tu préfèrerais un mock de systemd.journal.send ?

PS: ça n'est pas vraiment possible, systemd.journal.send est une valeur par défaut d'un argument du constructeur de JournalHandler, je ne peux pas le mocker, la valeur mockée ne sera pas prise en compte par le constructeur car elle est copié par valeur dans la définition de JournalHandler.__init__.

D'ac pas de souci (c'est le Reader que j'aurais aimé voir conservé, pour être sûr qu'en plus de l'écriture, il n'y a pas non plus d'erreur à la lecture des journaux avec la lib python{,3}-systemd).

#13 Updated by Benjamin Dauvergne about 2 months ago

Paul Marillonnet a écrit :

D'ac pas de souci (c'est le Reader que j'aurais aimé voir conservé, pour être sûr qu'en plus de l'écriture, il n'y a pas non plus d'erreur à la lecture des journaux avec la lib python{,3}-systemd).

Ça ne nous concerne plus là, c'était par facilité (finalement pas une bonne idée) que je passais par cette API, mais c'est le problème de python-systemd.

#14 Updated by Frédéric Péters about 2 months ago

  • Status changed from Résolu (à déployer) to Solution déployée

Also available in: Atom PDF