From 240c6913ab5e4d2412a41d3de38b784cbf7e5e1b Mon Sep 17 00:00:00 2001 From: Enrico Zini Date: Tue, 21 Apr 2020 22:08:51 +0200 Subject: [PATCH 1/4] Start using Inconsistency in CheckLDAPConsistency. refs: #5 --- dsa/housekeeping.py | 80 ++++++++++++++++++++++------------ dsa/tests/test_housekeeping.py | 39 ++++++++--------- sitechecks/housekeeping.py | 1 + sitechecks/models.py | 1 + 4 files changed, 71 insertions(+), 50 deletions(-) diff --git a/dsa/housekeeping.py b/dsa/housekeeping.py index d7f5ec7..93b50b5 100644 --- a/dsa/housekeeping.py +++ b/dsa/housekeeping.py @@ -6,6 +6,7 @@ from django.utils.timezone import now from backend.housekeeping import MakeLink, Housekeeper from . import udldap from backend import const +from sitechecks.models import Inconsistency import backend.models as bmodels import process.models as pmodels import backend.ops as bops @@ -140,8 +141,10 @@ class CheckLDAPConsistency(hk.Task): fpr = entry.single("keyFingerPrint") if fpr: - log.warn("%s: %s has fingerprint %s and gid %s in LDAP, but is not in our db", - self.IDENTIFIER, entry.uid, fpr, entry.single("gidNumber")) + Inconsistency.objects.found( + self.IDENTIFIER, + f"{entry.uid} has fingerprint {fpr} and gid {entry.single('gidNumber')} in LDAP," + " but is not in our db") else: ldap_fields_args = { "cn": entry.single("cn"), @@ -199,9 +202,12 @@ class CheckLDAPConsistency(hk.Task): elif not entry.is_dd: # mom040267 has a locked account (see the FD comments on the site) if person.ldap_fields.uid != "mom040267": - log.warn("%s: %s has accountStatus '%s' (comment: %s) but in our db the state is %s", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment"), const.ALL_STATUS_DESCS[person.status]) + Inconsistency.objects.found( + self.IDENTIFIER, + f"person has accountStatus '{entry.single('accountStatus')}'" + f" (comment: {entry.single('accountComment')})" + f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]}", + person=person) if entry.is_dd: # if person.status not in (const.STATUS_REMOVED_DD, const.STATUS_EMERITUS_DD): @@ -210,26 +216,36 @@ class CheckLDAPConsistency(hk.Task): if person.status_changed > self.email_forwarding_cutoff: if dsa_status == "retiring" and person.status != const.STATUS_EMERITUS_DD: - log.warn("%s: %s has accountStatus '%s' (comment: %s) but in our db the state is %s [retiring]", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment"), const.ALL_STATUS_DESCS[person.status]) + Inconsistency.objects.found( + self.IDENTIFIER, + f"person has accountStatus '{entry.single('accountStatus')}'" + f" (comment: {entry.single('accountComment')})" + f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]} [retiring]", + person=person) if dsa_status == "inactive" and person.status != const.STATUS_REMOVED_DD: - log.warn("%s: %s has accountStatus '%s' (comment: %s) but in our db the state is %s [inactive]", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment"), const.ALL_STATUS_DESCS[person.status]) + Inconsistency.objects.found( + self.IDENTIFIER, + f"person has accountStatus '{entry.single('accountStatus')}'" + f" (comment: {entry.single('accountComment')})" + f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]} [inactive]", + person=person) if dsa_status == "locked": parsed = entry.single("accountStatus").split() if len(parsed) == 1: - log.warn("%s: %s has accountStatus '%s' (comment: %s) locked with no date", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment")) + Inconsistency.objects.found( + self.IDENTIFIER, + f"person has accountStatus '{entry.single('accountStatus')}'" + f"(comment: {entry.single('accountComment')}) locked with no date", + person=person) else: if dsa_status_date is not None and dsa_status_date < self.email_forwarding_cutoff.date(): - log.warn("%s: %s has accountStatus '%s' (comment: %s) locked for longer than 6 months", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment")) + Inconsistency.objects.found( + self.IDENTIFIER, + f"person has accountStatus '{entry.single('accountStatus')}'" + f" (comment: {entry.single('accountComment')}) locked for longer than 6 months", + person=person) def run_main(self, stage): # Prefetch people and index them by uid @@ -253,15 +269,19 @@ class CheckLDAPConsistency(hk.Task): try: dsa_status_date = datetime.datetime.strptime(parsed[1], "%Y-%m-%d").date() except ValueError: - log.warn("%s: %s has accountStatus '%s' (comment: %s) with unparsable date", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment")) + Inconsistency.objects.found( + self.IDENTIFIER, + "person has accountStatus '{entry.single('accountStatus')}'" + " (comment: {entry.single('accountComment')}) with unparsable date", + person=person) dsa_status_date = None else: dsa_status_date = None if dsa_status not in ("retiring", "inactive", "memorial", "locked", "renamed"): - log.warn("%s: %s has unknown accountStatus %s", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus")) + Inconsistency.objects.found( + self.IDENTIFIER, + "person has unknown accountStatus {entry.single('accountStatus')}", + person=person) else: self.check_account_status(person, entry, dsa_status, dsa_status_date) else: @@ -269,14 +289,18 @@ class CheckLDAPConsistency(hk.Task): # DDs we don't expect if entry.is_dd: if person.status not in (const.STATUS_DD_U, const.STATUS_DD_NU): - log.warn("%s: %s has supplementaryGid 'Debian', but in our db the state is %s", - self.IDENTIFIER, self.hk.link(person), - const.ALL_STATUS_DESCS[person.status]) + Inconsistency.objects.found( + self.IDENTIFIER, + "person has supplementaryGid 'Debian'," + f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]}", + person=person) else: if person.status in (const.STATUS_DD_U, const.STATUS_DD_NU): - log.warn("%s: %s has no supplementaryGid 'Debian', but in our db the state is %s", - self.IDENTIFIER, self.hk.link(person), - const.ALL_STATUS_DESCS[person.status]) + Inconsistency.objects.found( + self.IDENTIFIER, + "person has no supplementaryGid 'Debian'," + f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]}", + person=person) email = entry.single("emailForward") if email != person.ldap_fields.email: diff --git a/dsa/tests/test_housekeeping.py b/dsa/tests/test_housekeeping.py index b591a70..2862cc9 100644 --- a/dsa/tests/test_housekeeping.py +++ b/dsa/tests/test_housekeeping.py @@ -16,6 +16,12 @@ class MockHousekeeper: def link(self, person): return str(person.ldap_fields.uid or person.pk) + def run(self, cls, stage: str = "main"): + task = cls(self) + task.IDENTIFIER = f"test.{cls.__name__}" + getattr(task, f"run_{stage}")(None) + return task + class MockEntry: def __init__(self, uid, **attrs): @@ -41,8 +47,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): mn="tmn", sn="tsn", emailForward="test@example.org", accountStatus=None) ] with LogCapture() as lc: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) lc.check(("dsa.housekeeping", "WARNING", "None: newdd: created to mirror a removed DD account from LDAP")) p = Person.objects.get(ldap_fields__uid="newdd") @@ -65,8 +70,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): sn="tsn", emailForward="test@example.org", accountStatus=None) ] with LogCapture() as lc: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) lc.check(("dsa.housekeeping", "WARNING", "None: newguest: created to mirror a removed guest account from LDAP")) @@ -91,8 +95,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): accountStatus=None) ] with LogCapture() as lc: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) lc.check(("dsa.housekeeping", "WARNING", "None: newdd has fingerprint 66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB and gid 800 in LDAP," " but is not in our db")) @@ -107,8 +110,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): mn="tmn", sn="tsn", emailForward="test@example.org", accountStatus=None) ] with LogCapture() as lc: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) lc.check( ("dsa.housekeeping", "INFO", "None: dd_u changing email_ldap from dd_u@example.org to test@example.org" " (source: LDAP)"), @@ -138,8 +140,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): keyFingerPrint="66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB", accountStatus=None) ] with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "WARNING:dsa.housekeeping:" "None: dm has supplementaryGid 'Debian', but in our db the state is Debian Maintainer", @@ -157,8 +158,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): accountStatus="inactive 2018-03-20"), ] with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "WARNING:dsa.housekeeping:" "None: dd_u has accountStatus 'inactive 2018-03-20' (comment: None)" @@ -178,8 +178,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): accountStatus="inactive 2018-03-20", accountComment="RT#1234"), ] with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "WARNING:dsa.housekeeping:None: dd_u has accountStatus 'inactive 2018-03-20' " '(comment: RT#1234) but in our db the state is Debian Developer, uploading [inactive]']) @@ -197,8 +196,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): emailForward="dd_u@example.org", accountStatus="retiring 2018-03-20"), ] with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "WARNING:dsa.housekeeping:None: dd_u has accountStatus 'retiring 2018-03-20' " '(comment: None) but in our db the state is Debian Developer, uploading ' @@ -209,8 +207,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): process.approved_time = now() process.save() with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "INFO:dsa.housekeeping:None: dd_u closed from dsa: retiring 2018-03-20", ]) @@ -234,8 +231,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): emailForward="test@example.org", accountStatus="retiring 2018-03-20"), ] with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "WARNING:dsa.housekeeping:None: dd_u has accountStatus 'retiring 2018-03-20' " '(comment: None) but in our db the state is Debian Developer, uploading ' @@ -248,8 +244,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): process.approved_time = now() process.save() with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "INFO:dsa.housekeeping:None: dd_u closed from dsa: retiring 2018-03-20", ]) diff --git a/sitechecks/housekeeping.py b/sitechecks/housekeeping.py index 80dfc52..33b308c 100644 --- a/sitechecks/housekeeping.py +++ b/sitechecks/housekeeping.py @@ -1,3 +1,4 @@ +from __future__ import annotations from collections import defaultdict import logging import django_housekeeping as hk diff --git a/sitechecks/models.py b/sitechecks/models.py index ac449c1..9af63a8 100644 --- a/sitechecks/models.py +++ b/sitechecks/models.py @@ -1,3 +1,4 @@ +from __future__ import annotations from django.db import models from backend.models import Person from process.models import Process -- GitLab From f57c7054d473a4ca2989f387e0341119d2358236 Mon Sep 17 00:00:00 2001 From: Enrico Zini Date: Tue, 21 Apr 2020 22:44:19 +0200 Subject: [PATCH 2/4] Fixed some of the tests to look for Inconsistency records instead of logs. refs: #5 --- dsa/tests/test_housekeeping.py | 86 ++++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 34 deletions(-) diff --git a/dsa/tests/test_housekeeping.py b/dsa/tests/test_housekeeping.py index 2862cc9..aefb5f0 100644 --- a/dsa/tests/test_housekeeping.py +++ b/dsa/tests/test_housekeeping.py @@ -5,6 +5,7 @@ from dsa.housekeeping import CheckLDAPConsistency from backend.models import Person import backend.const as const from process.models import Process +from sitechecks.models import Inconsistency from unittest import mock from testfixtures import LogCapture @@ -40,15 +41,28 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): super().setUp() self.hk = MockHousekeeper(self.persons.oldam) + def assertInconsistenciesEqual(self, items): + res = list(Inconsistency.objects.all()) + self.assertEqual(len(res), len(items)) + for inc, item in zip(res, items): + person = item.get("person") + if person is not None: + self.assertEqual(inc.person, person) + text = item.get("text") + if text is not None: + self.assertEqual(inc.text, text) + def test_new_removed(self): with mock.patch("dsa.udldap.list_people") as m: m.return_value = [ MockEntry("newdd", supplementaryGid=["Debian"], cn="tcn", mn="tmn", sn="tsn", emailForward="test@example.org", accountStatus=None) ] - with LogCapture() as lc: - self.hk.run(CheckLDAPConsistency) - lc.check(("dsa.housekeeping", "WARNING", "None: newdd: created to mirror a removed DD account from LDAP")) + self.hk.run(CheckLDAPConsistency) + self.assertInconsistenciesEqual([ + {"person": self.persons.newdd, + "text": "newdd: created to mirror a removed DD account from LDAP"}, + ]) p = Person.objects.get(ldap_fields__uid="newdd") audit = ["{}:{}".format(l.author.lookup_key, l.notes) for l in p.audit_log.all()] @@ -69,10 +83,10 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): MockEntry("newguest", supplementaryGid=[], cn="tcn", mn="tmn", sn="tsn", emailForward="test@example.org", accountStatus=None) ] - with LogCapture() as lc: + with self.assertLogs() as log: self.hk.run(CheckLDAPConsistency) - lc.check(("dsa.housekeeping", "WARNING", - "None: newguest: created to mirror a removed guest account from LDAP")) + self.assertEqual(log.output, [ + "WARNING:dsa.housekeeping:test.CheckLDAPConsistency: newguest: created to mirror a removed guest account from LDAP"]) p = Person.objects.get(ldap_fields__uid="newguest") audit = ["{}:{}".format(l.author.lookup_key, l.notes) for l in p.audit_log.all()] @@ -94,11 +108,12 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): emailForward="test@example.org", keyFingerPrint="66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB", accountStatus=None) ] - with LogCapture() as lc: - self.hk.run(CheckLDAPConsistency) - lc.check(("dsa.housekeeping", "WARNING", - "None: newdd has fingerprint 66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB and gid 800 in LDAP," - " but is not in our db")) + self.hk.run(CheckLDAPConsistency) + self.assertInconsistenciesEqual([ + {"person": self.persons.newdd, + "text": "person has fingerprint 66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB and gid 800 in LDAP," + " but is not in our db"}, + ]) with self.assertRaises(Person.DoesNotExist): Person.objects.get(ldap_fields__uid="newdd") @@ -139,11 +154,10 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): MockEntry("dm", supplementaryGid=["Debian"], cn="Dm", mn="", sn="", emailForward="dm@example.org", keyFingerPrint="66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB", accountStatus=None) ] - with self.assertLogs() as log: - self.hk.run(CheckLDAPConsistency) - self.assertEqual(log.output, [ - "WARNING:dsa.housekeeping:" - "None: dm has supplementaryGid 'Debian', but in our db the state is Debian Maintainer", + self.hk.run(CheckLDAPConsistency) + self.assertInconsistenciesEqual([ + {"person": self.persons.dm, + "text": "person has supplementaryGid 'Debian', but in our db the state is Debian Maintainer"}, ]) p = self.persons.dm @@ -157,12 +171,13 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): emailForward="dd_u@example.org", keyFingerPrint="66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB", accountStatus="inactive 2018-03-20"), ] - with self.assertLogs() as log: - self.hk.run(CheckLDAPConsistency) - self.assertEqual(log.output, [ - "WARNING:dsa.housekeeping:" - "None: dd_u has accountStatus 'inactive 2018-03-20' (comment: None)" - " but in our db the state is Debian Developer, uploading [inactive]", + self.hk.run(CheckLDAPConsistency) + + self.assertInconsistenciesEqual([ + {"person": self.persons.dd_u, + "text": + "person has accountStatus 'inactive 2018-03-20' (comment: None)" + " but in our db the state is Debian Developer, uploading [inactive]"} ]) p = self.persons.dd_u @@ -177,11 +192,14 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): keyFingerPrint="66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB", accountStatus="inactive 2018-03-20", accountComment="RT#1234"), ] - with self.assertLogs() as log: - self.hk.run(CheckLDAPConsistency) - self.assertEqual(log.output, [ - "WARNING:dsa.housekeeping:None: dd_u has accountStatus 'inactive 2018-03-20' " - '(comment: RT#1234) but in our db the state is Debian Developer, uploading [inactive]']) + + self.hk.run(CheckLDAPConsistency) + self.assertInconsistenciesEqual([ + {"person": self.persons.dd_u, + "text": + "person has accountStatus 'inactive 2018-03-20' " + '(comment: RT#1234) but in our db the state is Debian Developer, uploading [inactive]'} + ]) p = self.persons.dd_u p.refresh_from_db() @@ -195,12 +213,12 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): MockEntry("dd_u", supplementaryGid=["Debian"], cn="Dd_u", mn="", sn="", emailForward="dd_u@example.org", accountStatus="retiring 2018-03-20"), ] - with self.assertLogs() as log: - self.hk.run(CheckLDAPConsistency) - self.assertEqual(log.output, [ - "WARNING:dsa.housekeeping:None: dd_u has accountStatus 'retiring 2018-03-20' " - '(comment: None) but in our db the state is Debian Developer, uploading ' - '[retiring]', + self.hk.run(CheckLDAPConsistency) + self.assertInconsistenciesEqual([ + {"person": self.persons.dd_u, + "text": "person has accountStatus 'retiring 2018-03-20' " + '(comment: None) but in our db the state is Debian Developer, uploading ' + '[retiring]'}, ]) process.approved_by = self.persons.dam @@ -209,7 +227,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): with self.assertLogs() as log: self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ - "INFO:dsa.housekeeping:None: dd_u closed from dsa: retiring 2018-03-20", + "INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u closed from dsa: retiring 2018-03-20", ]) p = self.persons.dd_u -- GitLab From dae96feb81dba216c6481fbe8c4aee29d4c4687f Mon Sep 17 00:00:00 2001 From: Enrico Zini Date: Tue, 21 Apr 2020 22:46:21 +0200 Subject: [PATCH 3/4] Removed the last of textfixtures uses. refs: #5 --- .gitlab-ci.yml | 1 - dsa/tests/test_housekeeping.py | 17 ++++++++--------- test-instance/deploy.yml | 1 - 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a214d69..c7b8c85 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -15,7 +15,6 @@ gnupg debian-keyring python3-git - python3-testfixtures libjs-bootstrap4 fonts-fork-awesome libjs-jquery-datatables diff --git a/dsa/tests/test_housekeeping.py b/dsa/tests/test_housekeeping.py index aefb5f0..407834a 100644 --- a/dsa/tests/test_housekeeping.py +++ b/dsa/tests/test_housekeeping.py @@ -7,7 +7,6 @@ import backend.const as const from process.models import Process from sitechecks.models import Inconsistency from unittest import mock -from testfixtures import LogCapture class MockHousekeeper: @@ -124,15 +123,15 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): MockEntry("dd_u", supplementaryGid=["Debian"], cn="tcn", mn="tmn", sn="tsn", emailForward="test@example.org", accountStatus=None) ] - with LogCapture() as lc: + with self.assertLogs() as log: self.hk.run(CheckLDAPConsistency) - lc.check( - ("dsa.housekeeping", "INFO", "None: dd_u changing email_ldap from dd_u@example.org to test@example.org" - " (source: LDAP)"), - ('dsa.housekeeping', 'INFO', 'None: dd_u changing cn from Dd_u to tcn (source: LDAP)'), - ('dsa.housekeeping', 'INFO', 'None: dd_u changing mn from to tmn (source: LDAP)'), - ('dsa.housekeeping', 'INFO', 'None: dd_u changing sn from to tsn (source: LDAP)'), - ) + self.assertEqual(log.output, [ + "INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u changing email_ldap from dd_u@example.org" + " to test@example.org (source: LDAP)", + 'INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u changing cn from Dd_u to tcn (source: LDAP)', + 'INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u changing mn from to tmn (source: LDAP)', + 'INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u changing sn from to tsn (source: LDAP)', + ]) p = self.persons.dd_u p.refresh_from_db() diff --git a/test-instance/deploy.yml b/test-instance/deploy.yml index bef5f9a..74f5476 100644 --- a/test-instance/deploy.yml +++ b/test-instance/deploy.yml @@ -48,7 +48,6 @@ - gnupg - debian-keyring - python3-git - - python3-testfixtures - libjs-bootstrap4 - fonts-fork-awesome - libjs-jquery-datatables -- GitLab From 769cc8c933a79efb9bfcd9a6b2e71e4499e4b4d5 Mon Sep 17 00:00:00 2001 From: Enrico Zini Date: Tue, 21 Apr 2020 22:51:16 +0200 Subject: [PATCH 4/4] Fixed the remaining tests. refs: #5 --- dsa/tests/test_housekeeping.py | 41 ++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/dsa/tests/test_housekeeping.py b/dsa/tests/test_housekeeping.py index 407834a..35126f2 100644 --- a/dsa/tests/test_housekeeping.py +++ b/dsa/tests/test_housekeeping.py @@ -44,12 +44,10 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): res = list(Inconsistency.objects.all()) self.assertEqual(len(res), len(items)) for inc, item in zip(res, items): - person = item.get("person") - if person is not None: - self.assertEqual(inc.person, person) - text = item.get("text") - if text is not None: - self.assertEqual(inc.text, text) + if "person" in item: + self.assertEqual(inc.person, item["person"]) + if "text" in item: + self.assertEqual(inc.text, item["text"]) def test_new_removed(self): with mock.patch("dsa.udldap.list_people") as m: @@ -57,10 +55,11 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): MockEntry("newdd", supplementaryGid=["Debian"], cn="tcn", mn="tmn", sn="tsn", emailForward="test@example.org", accountStatus=None) ] - self.hk.run(CheckLDAPConsistency) - self.assertInconsistenciesEqual([ - {"person": self.persons.newdd, - "text": "newdd: created to mirror a removed DD account from LDAP"}, + with self.assertLogs() as log: + self.hk.run(CheckLDAPConsistency) + self.assertEqual(log.output, [ + "WARNING:dsa.housekeeping:test.CheckLDAPConsistency:" + " newdd: created to mirror a removed DD account from LDAP", ]) p = Person.objects.get(ldap_fields__uid="newdd") @@ -85,7 +84,8 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): with self.assertLogs() as log: self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ - "WARNING:dsa.housekeeping:test.CheckLDAPConsistency: newguest: created to mirror a removed guest account from LDAP"]) + "WARNING:dsa.housekeeping:test.CheckLDAPConsistency:" + " newguest: created to mirror a removed guest account from LDAP"]) p = Person.objects.get(ldap_fields__uid="newguest") audit = ["{}:{}".format(l.author.lookup_key, l.notes) for l in p.audit_log.all()] @@ -109,8 +109,8 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): ] self.hk.run(CheckLDAPConsistency) self.assertInconsistenciesEqual([ - {"person": self.persons.newdd, - "text": "person has fingerprint 66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB and gid 800 in LDAP," + {"person": None, + "text": "newdd has fingerprint 66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB and gid 800 in LDAP," " but is not in our db"}, ]) @@ -250,11 +250,14 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): with self.assertLogs() as log: self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ - "WARNING:dsa.housekeeping:None: dd_u has accountStatus 'retiring 2018-03-20' " - '(comment: None) but in our db the state is Debian Developer, uploading ' - '[retiring]', - "INFO:dsa.housekeeping:None: dd_u changing email_ldap from dd_u@example.org to test@example.org" - " (source: LDAP)", + "INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u changing email_ldap" + " from dd_u@example.org to test@example.org (source: LDAP)", + ]) + self.assertInconsistenciesEqual([ + {"person": self.persons.dd_u, + "text": "person has accountStatus 'retiring 2018-03-20' " + '(comment: None) but in our db the state is Debian Developer, uploading ' + '[retiring]'}, ]) process.approved_by = self.persons.dam @@ -263,7 +266,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): with self.assertLogs() as log: self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ - "INFO:dsa.housekeeping:None: dd_u closed from dsa: retiring 2018-03-20", + "INFO:dsa.housekeeping:test.CheckLDAPConsistency: dd_u closed from dsa: retiring 2018-03-20", ]) p = self.persons.dd_u -- GitLab