From bf2b9c68f96d19bb59a5bd49879b0a5e7d08cbbf Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 19 Jul 2025 15:35:40 +0200 Subject: [PATCH 01/16] refactor: drop unused code_coverage_collection var --- ietf/utils/test_runner.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/ietf/utils/test_runner.py b/ietf/utils/test_runner.py index ded812812a..a94bffa006 100644 --- a/ietf/utils/test_runner.py +++ b/ietf/utils/test_runner.py @@ -98,7 +98,6 @@ old_create: Optional[Callable] = None template_coverage_collection = None -code_coverage_collection = None url_coverage_collection = None validation_settings = {"validate_html": None, "validate_html_harder": None, "show_logging": False} @@ -460,18 +459,15 @@ def save_test_results(failures, test_labels): def set_coverage_checking(flag=True): global template_coverage_collection - global code_coverage_collection global url_coverage_collection if settings.SERVER_MODE == 'test' and settings.TEST_CODE_COVERAGE_CHECKER is not None: if flag: settings.TEST_CODE_COVERAGE_CHECKER.collector.resume() template_coverage_collection = True - code_coverage_collection = True url_coverage_collection = True else: settings.TEST_CODE_COVERAGE_CHECKER.collector.pause() template_coverage_collection = False - code_coverage_collection = False url_coverage_collection = False class CoverageReporter(Reporter): @@ -1135,9 +1131,8 @@ def _extra_tests(self): ), ] if self.check_coverage: - global template_coverage_collection, code_coverage_collection, url_coverage_collection + global template_coverage_collection, url_coverage_collection template_coverage_collection = True - code_coverage_collection = True url_coverage_collection = True tests += [ PyFlakesTestCase(test_runner=self, methodName='pyflakes_test'), From c98c3618816f14159043327f1c6c868df0c4b85f Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 19 Jul 2025 16:02:56 +0200 Subject: [PATCH 02/16] refactor: @skip_coverage -> pragma: no cover --- ietf/meeting/tests_views.py | 4 +--- ietf/utils/decorators.py | 11 ----------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index 444059b77f..bf26122279 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -54,7 +54,6 @@ from ietf.meeting.views import session_draft_list, parse_agenda_filter_params, sessions_post_save, agenda_extract_schedule from ietf.meeting.views import get_summary_by_area, get_summary_by_type, get_summary_by_purpose, generate_agenda_data from ietf.name.models import SessionStatusName, ImportantDateName, RoleName, ProceedingsMaterialTypeName -from ietf.utils.decorators import skip_coverage from ietf.utils.mail import outbox, empty_outbox, get_payload_text from ietf.utils.test_runner import TestBlobstoreManager from ietf.utils.test_utils import TestCase, login_testing_unauthorized, unicontent @@ -1168,8 +1167,7 @@ def test_session_draft_tarfile(self): os.unlink(filename) @skipIf(skip_pdf_tests, skip_message) - @skip_coverage - def test_session_draft_pdf(self): + def test_session_draft_pdf(self): # pragma: no cover session, filenames = self.build_session_setup() try: url = urlreverse('ietf.meeting.views.session_draft_pdf', kwargs={'num':session.meeting.number,'acronym':session.group.acronym}) diff --git a/ietf/utils/decorators.py b/ietf/utils/decorators.py index 56c28c4b19..f39c33253e 100644 --- a/ietf/utils/decorators.py +++ b/ietf/utils/decorators.py @@ -19,17 +19,6 @@ from ietf.person.models import Person, PersonalApiKey, PersonApiKeyEvent from ietf.utils import log -def skip_coverage(f): - @wraps(f) - def _wrapper(*args, **kwargs): - if settings.TEST_CODE_COVERAGE_CHECKER: - set_coverage_checking(False) - result = f(*args, **kwargs) - set_coverage_checking(True) - return result - else: - return f(*args, **kwargs) - return _wrapper def person_required(f): @wraps(f) From 9cc565fea4f1a6a42b5d568038173060f512b6b5 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 19 Jul 2025 16:03:42 +0200 Subject: [PATCH 03/16] chore(deps): bump coverage to current ver --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 4eb573ce36..aca1ba8a06 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,7 @@ boto3>=1.35,<1.36 boto3-stubs[s3]>=1.35,<1.36 botocore>=1.35,<1.36 celery>=5.2.6 -coverage>=4.5.4,<5.0 # Coverage 5.x moves from a json database to SQLite. Moving to 5.x will require substantial rewrites in ietf.utils.test_runner and ietf.release.views +coverage>=7.9.2 defusedxml>=0.7.1 # for TastyPie when using xml; not a declared dependency Django>4.2,<5 django-admin-rangefilter>=0.13.2 From 038dd2d13bb428a8d3f37d2aee728bb66ae5795f Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 19 Jul 2025 17:05:40 +0200 Subject: [PATCH 04/16] refactor: split up set_coverage_checking() --- ietf/utils/test_runner.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/ietf/utils/test_runner.py b/ietf/utils/test_runner.py index a94bffa006..7193733780 100644 --- a/ietf/utils/test_runner.py +++ b/ietf/utils/test_runner.py @@ -97,8 +97,8 @@ old_destroy: Optional[Callable] = None old_create: Optional[Callable] = None -template_coverage_collection = None -url_coverage_collection = None +template_coverage_collection = False +url_coverage_collection = False validation_settings = {"validate_html": None, "validate_html_harder": None, "show_logging": False} def start_vnu_server(port=8888): @@ -457,18 +457,19 @@ def save_test_results(failures, test_labels): tfile.write("%s OK\n" % (timestr, )) tfile.close() -def set_coverage_checking(flag=True): + +def set_template_coverage(flag): global template_coverage_collection + orig = template_coverage_collection + template_coverage_collection = flag + return orig + + +def set_url_coverage(flag): global url_coverage_collection - if settings.SERVER_MODE == 'test' and settings.TEST_CODE_COVERAGE_CHECKER is not None: - if flag: - settings.TEST_CODE_COVERAGE_CHECKER.collector.resume() - template_coverage_collection = True - url_coverage_collection = True - else: - settings.TEST_CODE_COVERAGE_CHECKER.collector.pause() - template_coverage_collection = False - url_coverage_collection = False + orig = url_coverage_collection + url_coverage_collection = flag + return orig class CoverageReporter(Reporter): def report(self): From e701bb3cafaba00874416d4acb932505adc5919b Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 19 Jul 2025 17:17:26 +0200 Subject: [PATCH 05/16] refactor: inline IetfLiveServerTestCase (there's only one subclass) --- ietf/utils/jstest.py | 41 +++++++++++++++++++++++++++++++++++---- ietf/utils/test_runner.py | 31 ----------------------------- 2 files changed, 37 insertions(+), 35 deletions(-) diff --git a/ietf/utils/jstest.py b/ietf/utils/jstest.py index 215d78d65f..cf242fc4eb 100644 --- a/ietf/utils/jstest.py +++ b/ietf/utils/jstest.py @@ -3,6 +3,8 @@ import os +from django.conf import settings +from django.contrib.staticfiles.testing import StaticLiveServerTestCase from django.urls import reverse as urlreverse from unittest import skipIf @@ -21,7 +23,11 @@ from ietf.utils.pipe import pipe -from ietf.utils.test_runner import IetfLiveServerTestCase +from ietf.utils.test_runner import ( + set_template_coverage, + set_url_coverage, + load_and_run_fixtures, +) executable_name = 'geckodriver' code, out, err = pipe('{} --version'.format(executable_name)) @@ -49,17 +55,44 @@ def ifSeleniumEnabled(func): return skipIf(skip_selenium, skip_message)(func) -class IetfSeleniumTestCase(IetfLiveServerTestCase): +class IetfSeleniumTestCase(StaticLiveServerTestCase): # pragma: no cover login_view = 'ietf.ietfauth.views.login' + @classmethod + def setUpClass(cls): + set_template_coverage(False) + set_url_coverage(False) + super().setUpClass() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + set_template_coverage(True) + set_url_coverage(True) + def setUp(self): - super(IetfSeleniumTestCase, self).setUp() + super().setUp() + # LiveServerTestCase uses TransactionTestCase which seems to + # somehow interfere with the fixture loading process in + # IetfTestRunner when running multiple tests (the first test + # is fine, in the next ones the fixtures have been wiped) - + # this is no doubt solvable somehow, but until then we simply + # recreate them here + from ietf.person.models import Person + if not Person.objects.exists(): + load_and_run_fixtures(verbosity=0) + self.replaced_settings = dict() + if hasattr(settings, 'IDTRACKER_BASE_URL'): + self.replaced_settings['IDTRACKER_BASE_URL'] = settings.IDTRACKER_BASE_URL + settings.IDTRACKER_BASE_URL = self.live_server_url self.driver = start_web_driver() self.driver.set_window_size(1024,768) def tearDown(self): - super(IetfSeleniumTestCase, self).tearDown() self.driver.close() + for k, v in self.replaced_settings.items(): + setattr(settings, k, v) + super().tearDown() def absreverse(self,*args,**kwargs): return '%s%s'%(self.live_server_url, urlreverse(*args, **kwargs)) diff --git a/ietf/utils/test_runner.py b/ietf/utils/test_runner.py index 7193733780..9863bdb7de 100644 --- a/ietf/utils/test_runner.py +++ b/ietf/utils/test_runner.py @@ -1217,37 +1217,6 @@ def run_tests(self, test_labels, extra_tests=None, **kwargs): return failures -class IetfLiveServerTestCase(StaticLiveServerTestCase): - @classmethod - def setUpClass(cls): - set_coverage_checking(False) - super(IetfLiveServerTestCase, cls).setUpClass() - - def setUp(self): - super(IetfLiveServerTestCase, self).setUp() - # LiveServerTestCase uses TransactionTestCase which seems to - # somehow interfere with the fixture loading process in - # IetfTestRunner when running multiple tests (the first test - # is fine, in the next ones the fixtures have been wiped) - - # this is no doubt solvable somehow, but until then we simply - # recreate them here - from ietf.person.models import Person - if not Person.objects.exists(): - load_and_run_fixtures(verbosity=0) - self.replaced_settings = dict() - if hasattr(settings, 'IDTRACKER_BASE_URL'): - self.replaced_settings['IDTRACKER_BASE_URL'] = settings.IDTRACKER_BASE_URL - settings.IDTRACKER_BASE_URL = self.live_server_url - - @classmethod - def tearDownClass(cls): - super(IetfLiveServerTestCase, cls).tearDownClass() - set_coverage_checking(True) - - def tearDown(self): - for k, v in self.replaced_settings.items(): - setattr(settings, k, v) - super().tearDown() class TestBlobstoreManager(): # N.B. buckets and blobstore are intentional Class-level attributes From ca8d623243a4694a091cab8435b9f074424c766b Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 19 Jul 2025 17:36:30 +0200 Subject: [PATCH 06/16] feat: disable_coverage context mgr --- ietf/meeting/tests_views.py | 3 ++- ietf/utils/test_runner.py | 13 ++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index bf26122279..9536eae861 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -55,7 +55,7 @@ from ietf.meeting.views import get_summary_by_area, get_summary_by_type, get_summary_by_purpose, generate_agenda_data from ietf.name.models import SessionStatusName, ImportantDateName, RoleName, ProceedingsMaterialTypeName from ietf.utils.mail import outbox, empty_outbox, get_payload_text -from ietf.utils.test_runner import TestBlobstoreManager +from ietf.utils.test_runner import TestBlobstoreManager, disable_coverage from ietf.utils.test_utils import TestCase, login_testing_unauthorized, unicontent from ietf.utils.timezone import date_today, time_now @@ -1167,6 +1167,7 @@ def test_session_draft_tarfile(self): os.unlink(filename) @skipIf(skip_pdf_tests, skip_message) + @disable_coverage() def test_session_draft_pdf(self): # pragma: no cover session, filenames = self.build_session_setup() try: diff --git a/ietf/utils/test_runner.py b/ietf/utils/test_runner.py index 9863bdb7de..047384a60e 100644 --- a/ietf/utils/test_runner.py +++ b/ietf/utils/test_runner.py @@ -48,6 +48,8 @@ import subprocess import tempfile import copy +from contextlib import contextmanager + import boto3 import botocore.config import factory.random @@ -471,9 +473,6 @@ def set_url_coverage(flag): url_coverage_collection = flag return orig -class CoverageReporter(Reporter): - def report(self): - self.find_file_reporters(None) total = Numbers() result = {"coverage": 0.0, "covered": {}, "format": 5, } @@ -499,6 +498,14 @@ def report(self): raise result["coverage"] = total.pc_covered/100.0 return result +@contextmanager +def disable_coverage(): + """Context manager/decorator that disables template/url coverage""" + orig_template = set_template_coverage(False) + orig_url = set_url_coverage(False) + yield + set_template_coverage(orig_template) + set_url_coverage(orig_url) class CoverageTest(unittest.TestCase): From b36d093709e28d564642c1c9f4fb1d8042a1f481 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 19 Jul 2025 17:47:01 +0200 Subject: [PATCH 07/16] chore: remove unused import --- ietf/utils/decorators.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ietf/utils/decorators.py b/ietf/utils/decorators.py index f39c33253e..f9d027a83a 100644 --- a/ietf/utils/decorators.py +++ b/ietf/utils/decorators.py @@ -15,7 +15,6 @@ import debug # pyflakes:ignore -from ietf.utils.test_runner import set_coverage_checking from ietf.person.models import Person, PersonalApiKey, PersonApiKeyEvent from ietf.utils import log From b5da82025dd695d8ddd06b4244831c5407d93231 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 19 Jul 2025 17:47:46 +0200 Subject: [PATCH 08/16] refactor: set_coverage_checking -> disable_coverage --- ietf/submit/checkers.py | 57 ++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/ietf/submit/checkers.py b/ietf/submit/checkers.py index 89908748a7..e02b686576 100644 --- a/ietf/submit/checkers.py +++ b/ietf/submit/checkers.py @@ -18,7 +18,7 @@ from ietf.utils import tool_version from ietf.utils.log import log, assertion from ietf.utils.pipe import pipe -from ietf.utils.test_runner import set_coverage_checking +from ietf.utils.test_runner import disable_coverage class DraftSubmissionChecker(object): name = "" @@ -247,34 +247,33 @@ def check_file_txt(self, path): ) # yanglint - set_coverage_checking(False) # we can't count the following as it may or may not be run, depending on setup - if settings.SUBMIT_YANGLINT_COMMAND and os.path.exists(settings.YANGLINT_BINARY): - cmd_template = settings.SUBMIT_YANGLINT_COMMAND - command = [ w for w in cmd_template.split() if not '=' in w ][0] - cmd = cmd_template.format(model=path, rfclib=settings.SUBMIT_YANG_RFC_MODEL_DIR, tmplib=workdir, - draftlib=settings.SUBMIT_YANG_DRAFT_MODEL_DIR, ianalib=settings.SUBMIT_YANG_IANA_MODEL_DIR, - cataloglib=settings.SUBMIT_YANG_CATALOG_MODEL_DIR, ) - code, out, err = pipe(cmd) - out = out.decode('utf-8') - err = err.decode('utf-8') - if code > 0 or len(err.strip()) > 0: - err_lines = err.splitlines() - for line in err_lines: - if line.strip(): - try: - if 'err : ' in line: - errors += 1 - if 'warn: ' in line: - warnings += 1 - except ValueError: - pass - #passed = passed and code == 0 # For the submission tool. Yang checks always pass - message += "{version}: {template}:\n{output}\n".format( - version=tool_version[command], - template=cmd_template, - output=out + "No validation errors\n" if (code == 0 and len(err) == 0) else out + err, - ) - set_coverage_checking(True) + with disable_coverage(): # pragma: no cover + if settings.SUBMIT_YANGLINT_COMMAND and os.path.exists(settings.YANGLINT_BINARY): + cmd_template = settings.SUBMIT_YANGLINT_COMMAND + command = [ w for w in cmd_template.split() if not '=' in w ][0] + cmd = cmd_template.format(model=path, rfclib=settings.SUBMIT_YANG_RFC_MODEL_DIR, tmplib=workdir, + draftlib=settings.SUBMIT_YANG_DRAFT_MODEL_DIR, ianalib=settings.SUBMIT_YANG_IANA_MODEL_DIR, + cataloglib=settings.SUBMIT_YANG_CATALOG_MODEL_DIR, ) + code, out, err = pipe(cmd) + out = out.decode('utf-8') + err = err.decode('utf-8') + if code > 0 or len(err.strip()) > 0: + err_lines = err.splitlines() + for line in err_lines: + if line.strip(): + try: + if 'err : ' in line: + errors += 1 + if 'warn: ' in line: + warnings += 1 + except ValueError: + pass + #passed = passed and code == 0 # For the submission tool. Yang checks always pass + message += "{version}: {template}:\n{output}\n".format( + version=tool_version[command], + template=cmd_template, + output=out + "No validation errors\n" if (code == 0 and len(err) == 0) else out + err, + ) else: errors += 1 message += "No such file: %s\nPossible mismatch between extracted xym file name and returned module name?\n" % (path) From 2ad36b5d117f52dcd9a1c98d6310a68ecb4c9be2 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 19 Jul 2025 17:48:31 +0200 Subject: [PATCH 09/16] refactor: elim more set_coverage_checking --- ietf/utils/tests.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/ietf/utils/tests.py b/ietf/utils/tests.py index 872aa366b9..6ef2253eaf 100644 --- a/ietf/utils/tests.py +++ b/ietf/utils/tests.py @@ -55,7 +55,11 @@ decode_header_value, show_that_mail_was_sent, ) -from ietf.utils.test_runner import get_template_paths, set_coverage_checking +from ietf.utils.test_runner import ( + get_template_paths, + set_template_coverage, + set_url_coverage, +) from ietf.utils.test_utils import TestCase, unicontent from ietf.utils.text import parse_unicode from ietf.utils.timezone import timezone_not_near_midnight @@ -312,14 +316,15 @@ def qualified(name): return list(callbacks) -class TemplateChecksTestCase(TestCase): +class TemplateChecksTestCase(TestCase): # pragma: no cover paths = [] # type: List[str] templates = {} # type: Dict[str, Template] def setUp(self): super().setUp() - set_coverage_checking(False) + set_template_coverage(False) + set_url_coverage(False) self.paths = list(get_template_paths()) self.paths.sort() for path in self.paths: @@ -329,7 +334,8 @@ def setUp(self): pass def tearDown(self): - set_coverage_checking(True) + set_template_coverage(True) + set_url_coverage(True) super().tearDown() def test_parse_templates(self): From 1fe314dc84c34bca0cfcf0c7f4d05652fcef481d Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 19 Jul 2025 17:49:44 +0200 Subject: [PATCH 10/16] refactor: start using coverage 7.9.2 --- ietf/settings.py | 11 +----- ietf/settings_test.py | 6 +-- ietf/utils/test_runner.py | 78 +++++++++++++++++++++------------------ 3 files changed, 45 insertions(+), 50 deletions(-) diff --git a/ietf/settings.py b/ietf/settings.py index 8d548b7d09..e243a00e90 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -735,16 +735,7 @@ def skip_unreadable_post(record): TEST_COVERAGE_MAIN_FILE = os.path.join(BASE_DIR, "../release-coverage.json") TEST_COVERAGE_LATEST_FILE = os.path.join(BASE_DIR, "../latest-coverage.json") -TEST_CODE_COVERAGE_CHECKER = None -# TODO-PY312: figure out how to run coverage -# Context: the old version of coverage that we use (4.5.4, ca 2019) is falling back from its -# fast CTracer module to its very slow PyTracer when used on Python 3.12. It's not clear exactly -# why, but it's almost 3x slower. The situation may be better if we can update to a current -# version of coverage, but see https://github.com/nedbat/coveragepy/issues/1665 for more info. -# For now at least, disabling the checker completely. -# if SERVER_MODE != 'production': -# import coverage -# TEST_CODE_COVERAGE_CHECKER = coverage.Coverage(source=[ BASE_DIR ], cover_pylib=False, omit=TEST_CODE_COVERAGE_EXCLUDE_FILES) +TEST_CODE_COVERAGE_ENABLED = False TEST_CODE_COVERAGE_REPORT_PATH = "coverage/" TEST_CODE_COVERAGE_REPORT_URL = os.path.join(STATIC_URL, TEST_CODE_COVERAGE_REPORT_PATH, "index.html") diff --git a/ietf/settings_test.py b/ietf/settings_test.py index 9a42e8b99d..1b4b39c7a4 100755 --- a/ietf/settings_test.py +++ b/ietf/settings_test.py @@ -14,7 +14,7 @@ import shutil import tempfile from ietf.settings import * # pyflakes:ignore -from ietf.settings import TEST_CODE_COVERAGE_CHECKER, ORIG_AUTH_PASSWORD_VALIDATORS +from ietf.settings import ORIG_AUTH_PASSWORD_VALIDATORS import debug # pyflakes:ignore debug.debug = True @@ -52,9 +52,7 @@ def __getitem__(self, item): BLOBDB_DATABASE = "default" DATABASE_ROUTERS = [] # type: ignore -if TEST_CODE_COVERAGE_CHECKER and not TEST_CODE_COVERAGE_CHECKER._started: # pyflakes:ignore - TEST_CODE_COVERAGE_CHECKER.start() # pyflakes:ignore - +TEST_CODE_COVERAGE_ENABLED = True def tempdir_with_cleanup(**kwargs): """Utility to create a temporary dir and arrange cleanup""" diff --git a/ietf/utils/test_runner.py b/ietf/utils/test_runner.py index 047384a60e..d21c65d359 100644 --- a/ietf/utils/test_runner.py +++ b/ietf/utils/test_runner.py @@ -60,9 +60,7 @@ from typing import Callable, Optional from urllib.parse import urlencode -from coverage.report import Reporter -from coverage.results import Numbers -from coverage.misc import NotPython +import coverage import django from django.conf import settings @@ -103,6 +101,42 @@ url_coverage_collection = False validation_settings = {"validate_html": None, "validate_html_harder": None, "show_logging": False} + +class CoverageManager: + checker = None + + def start(self): + if ( + settings.SERVER_MODE == "test" + and settings.TEST_CODE_COVERAGE_ENABLED + and self.checker is None + ): + self.checker = coverage.Coverage( + source=[settings.BASE_DIR], + cover_pylib=False, + omit=settings.TEST_CODE_COVERAGE_EXCLUDE_FILES, + ) + self.checker.start() + self.enable() + + def stop(self): + if self.checker is not None: + self.checker.stop() + + def enable(self): + """Enable - tbd if this is possible""" + + def disable(self): + """Disable - tbd if this is possible""" + + def save(self): + if self.checker is not None: + self.checker.save() + + +coverage_manager = CoverageManager() + + def start_vnu_server(port=8888): "Start a vnu validation server on the indicated port" vnu = subprocess.Popen( @@ -474,30 +508,6 @@ def set_url_coverage(flag): return orig - total = Numbers() - result = {"coverage": 0.0, "covered": {}, "format": 5, } - for fr in self.file_reporters: - try: - analysis = self.coverage._analyze(fr) - nums = analysis.numbers - missing_nums = sorted(analysis.missing) - with io.open(analysis.filename, encoding='utf-8') as file: - lines = file.read().splitlines() - missing_lines = [ lines[l-1] for l in missing_nums ] - result["covered"][fr.relative_filename()] = (nums.n_statements, nums.pc_covered/100.0, missing_nums, missing_lines) - total += nums - except KeyboardInterrupt: # pragma: not covered - raise - except Exception: - report_it = not self.config.ignore_errors - if report_it: - typ, msg = sys.exc_info()[:2] - if typ is NotPython and not fr.should_be_python(): - report_it = False - if report_it: - raise - result["coverage"] = total.pc_covered/100.0 - return result @contextmanager def disable_coverage(): """Context manager/decorator that disables template/url coverage""" @@ -593,7 +603,7 @@ def ignore_pattern(regex, pattern): self.skipTest("Coverage switched off with --skip-coverage") def code_coverage_test(self): - if self.runner.check_coverage and self.runner.code_coverage_checker is not None: + if self.runner.check_coverage and settings.TEST_CODE_COVERAGE_ENABLED: include = [ os.path.join(path, '*') for path in self.runner.test_paths ] checker = self.runner.code_coverage_checker checker.stop() @@ -608,8 +618,8 @@ def code_coverage_test(self): if self.runner.run_full_test_suite and self.runner.html_report: checker.html_report(directory=settings.TEST_CODE_COVERAGE_REPORT_DIR) # In any case, build a dictionary with per-file data for this run - reporter = CoverageReporter(checker, checker.config) - self.runner.coverage_data["code"] = reporter.report() + with open("jlrcoverage.txt", "w") as f: + self.runner.coverage_data["code"] = checker.json_report(outfile=f) self.report_test_result("code") else: self.skipTest("Coverage switched off with --skip-coverage") @@ -833,12 +843,8 @@ def setup_test_environment(self, **kwargs): settings.MIDDLEWARE = ('ietf.utils.test_runner.record_urls_middleware',) + tuple(settings.MIDDLEWARE) - self.code_coverage_checker = settings.TEST_CODE_COVERAGE_CHECKER - if self.code_coverage_checker and not self.code_coverage_checker._started: - sys.stderr.write(" ** Warning: In %s: Expected the coverage checker to have\n" - " been started already, but it wasn't. Doing so now. Coverage numbers\n" - " will be off, though.\n" % __name__) - self.code_coverage_checker.start() + # start the coverage manager (if enabled) + coverage_manager.start() if settings.SITE_ID != 1: print(" Changing SITE_ID to '1' during testing.") From e2b0b5186fe9910c16db4c323ff716583c5ab8c1 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 26 Jul 2025 14:03:15 +0200 Subject: [PATCH 11/16] feat: working coverage 7.9 implementation * Extract coverage tools to ietf.utils.coverage * Revert to starting checker in settings_test Does not exactly match previous coverage reports. Need to investigate. --- ietf/settings.py | 5 ++- ietf/settings_test.py | 3 +- ietf/utils/coverage.py | 90 +++++++++++++++++++++++++++++++++++++++ ietf/utils/test_runner.py | 67 ++++++----------------------- 4 files changed, 110 insertions(+), 55 deletions(-) create mode 100644 ietf/utils/coverage.py diff --git a/ietf/settings.py b/ietf/settings.py index e243a00e90..2f7baa7a9a 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -735,7 +735,10 @@ def skip_unreadable_post(record): TEST_COVERAGE_MAIN_FILE = os.path.join(BASE_DIR, "../release-coverage.json") TEST_COVERAGE_LATEST_FILE = os.path.join(BASE_DIR, "../latest-coverage.json") -TEST_CODE_COVERAGE_ENABLED = False +TEST_CODE_COVERAGE_CHECKER = None +if SERVER_MODE != 'production': + from ietf.utils.coverage import CoverageManager + TEST_CODE_COVERAGE_CHECKER = CoverageManager() TEST_CODE_COVERAGE_REPORT_PATH = "coverage/" TEST_CODE_COVERAGE_REPORT_URL = os.path.join(STATIC_URL, TEST_CODE_COVERAGE_REPORT_PATH, "index.html") diff --git a/ietf/settings_test.py b/ietf/settings_test.py index 1b4b39c7a4..6479069db0 100755 --- a/ietf/settings_test.py +++ b/ietf/settings_test.py @@ -52,7 +52,8 @@ def __getitem__(self, item): BLOBDB_DATABASE = "default" DATABASE_ROUTERS = [] # type: ignore -TEST_CODE_COVERAGE_ENABLED = True +if TEST_CODE_COVERAGE_CHECKER: # pyflakes:ignore + TEST_CODE_COVERAGE_CHECKER.start() # pyflakes:ignore def tempdir_with_cleanup(**kwargs): """Utility to create a temporary dir and arrange cleanup""" diff --git a/ietf/utils/coverage.py b/ietf/utils/coverage.py new file mode 100644 index 0000000000..fa80df0713 --- /dev/null +++ b/ietf/utils/coverage.py @@ -0,0 +1,90 @@ +# Copyright The IETF Trust 2025, All Rights Reserved +from coverage import Coverage, CoverageData, FileReporter +from coverage.control import override_config as override_coverage_config +from coverage.results import Numbers +from coverage.report_core import get_analysis_to_report +from coverage.results import Analysis +from django.conf import settings + + +class CoverageManager: + checker: Coverage | None = None + started = False + + def start(self): + if settings.SERVER_MODE != "production" and not self.started: + self.checker = Coverage( + source=[settings.BASE_DIR], + cover_pylib=False, + omit=settings.TEST_CODE_COVERAGE_EXCLUDE_FILES, + ) + for exclude_regex in getattr( + settings, + "TEST_CODE_COVERAGE_EXCLUDE_LINES", + [], + ): + self.checker.exclude(exclude_regex) + self.checker.start() + self.started = True + + def stop(self): + if self.checker is not None: + self.checker.stop() + + def save(self): + if self.checker is not None: + self.checker.save() + + def report(self, include: list[str] | None = None): + if self.checker is None: + return None + reporter = CustomJsonReporter() + with override_coverage_config( + self.checker, + report_include=include, + ): + return reporter.report(self.checker) + + +class CustomJsonReporter: + total = Numbers() + + def report(self, coverage): + coverage_data = coverage.get_data() + coverage_data.set_query_contexts(None) + measured_files = {} + for file_reporter, analysis in get_analysis_to_report(coverage, None): + measured_files[file_reporter.relative_filename()] = self.report_one_file( + coverage_data, + analysis, + file_reporter, + ) + tot_numer, tot_denom = self.total.ratio_covered + return { + "coverage": 1 if tot_denom == 0 else tot_numer / tot_denom, + "covered": measured_files, + "format": 5, + } + + def report_one_file( + self, + coverage_data: CoverageData, + analysis: Analysis, + file_reporter: FileReporter, + ): + """Extract the relevant report data for a single file.""" + nums = analysis.numbers + self.total += nums + n_statements = nums.n_statements + numer, denom = nums.ratio_covered + fraction_covered = 1 if denom == 0 else numer / denom + missing_line_nums = sorted(analysis.missing) + # Extract missing lines from source files + source_lines = file_reporter.source().splitlines() + missing_lines = [source_lines[num - 1] for num in missing_line_nums] + return ( + n_statements, + fraction_covered, + missing_line_nums, + missing_lines, + ) diff --git a/ietf/utils/test_runner.py b/ietf/utils/test_runner.py index d21c65d359..f415a715cb 100644 --- a/ietf/utils/test_runner.py +++ b/ietf/utils/test_runner.py @@ -60,8 +60,6 @@ from typing import Callable, Optional from urllib.parse import urlencode -import coverage - import django from django.conf import settings from django.contrib.staticfiles.testing import StaticLiveServerTestCase @@ -102,41 +100,6 @@ validation_settings = {"validate_html": None, "validate_html_harder": None, "show_logging": False} -class CoverageManager: - checker = None - - def start(self): - if ( - settings.SERVER_MODE == "test" - and settings.TEST_CODE_COVERAGE_ENABLED - and self.checker is None - ): - self.checker = coverage.Coverage( - source=[settings.BASE_DIR], - cover_pylib=False, - omit=settings.TEST_CODE_COVERAGE_EXCLUDE_FILES, - ) - self.checker.start() - self.enable() - - def stop(self): - if self.checker is not None: - self.checker.stop() - - def enable(self): - """Enable - tbd if this is possible""" - - def disable(self): - """Disable - tbd if this is possible""" - - def save(self): - if self.checker is not None: - self.checker.save() - - -coverage_manager = CoverageManager() - - def start_vnu_server(port=8888): "Start a vnu validation server on the indicated port" vnu = subprocess.Popen( @@ -603,23 +566,24 @@ def ignore_pattern(regex, pattern): self.skipTest("Coverage switched off with --skip-coverage") def code_coverage_test(self): - if self.runner.check_coverage and settings.TEST_CODE_COVERAGE_ENABLED: - include = [ os.path.join(path, '*') for path in self.runner.test_paths ] - checker = self.runner.code_coverage_checker - checker.stop() + if ( + self.runner.check_coverage + and settings.TEST_CODE_COVERAGE_CHECKER is not None + ): + coverage_manager = settings.TEST_CODE_COVERAGE_CHECKER + coverage_manager.stop() # Save to the .coverage file - checker.save() + coverage_manager.save() # Apply the configured and requested omit and include data - checker.config.from_args(ignore_errors=None, omit=settings.TEST_CODE_COVERAGE_EXCLUDE_FILES, - include=include, file=None) - for pattern in settings.TEST_CODE_COVERAGE_EXCLUDE_LINES: - checker.exclude(pattern) # Maybe output an HTML report if self.runner.run_full_test_suite and self.runner.html_report: - checker.html_report(directory=settings.TEST_CODE_COVERAGE_REPORT_DIR) - # In any case, build a dictionary with per-file data for this run - with open("jlrcoverage.txt", "w") as f: - self.runner.coverage_data["code"] = checker.json_report(outfile=f) + coverage_manager.checker.html_report( + directory=settings.TEST_CODE_COVERAGE_REPORT_DIR + ) + # Generate the output report data + self.runner.coverage_data["code"] = coverage_manager.report( + include=[str(pathlib.Path(p) / "*") for p in self.runner.test_paths] + ) self.report_test_result("code") else: self.skipTest("Coverage switched off with --skip-coverage") @@ -843,9 +807,6 @@ def setup_test_environment(self, **kwargs): settings.MIDDLEWARE = ('ietf.utils.test_runner.record_urls_middleware',) + tuple(settings.MIDDLEWARE) - # start the coverage manager (if enabled) - coverage_manager.start() - if settings.SITE_ID != 1: print(" Changing SITE_ID to '1' during testing.") settings.SITE_ID = 1 From b948892fe72afc93e98f04d8c76e943517816394 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 28 Jul 2025 11:00:59 -0300 Subject: [PATCH 12/16] refactor: CustomJsonReporter->CustomDictReporter --- ietf/utils/coverage.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ietf/utils/coverage.py b/ietf/utils/coverage.py index fa80df0713..5e40c1edc7 100644 --- a/ietf/utils/coverage.py +++ b/ietf/utils/coverage.py @@ -38,7 +38,7 @@ def save(self): def report(self, include: list[str] | None = None): if self.checker is None: return None - reporter = CustomJsonReporter() + reporter = CustomDictReporter() with override_coverage_config( self.checker, report_include=include, @@ -46,7 +46,7 @@ def report(self, include: list[str] | None = None): return reporter.report(self.checker) -class CustomJsonReporter: +class CustomDictReporter: total = Numbers() def report(self, coverage): From 9f14a0f56a35967f08a86cf9516cf50261fc943d Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 28 Jul 2025 11:03:28 -0300 Subject: [PATCH 13/16] chore: remove "migration" coverage entry Has not been populated in quite some time --- ietf/utils/test_runner.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ietf/utils/test_runner.py b/ietf/utils/test_runner.py index f415a715cb..9be1bc19ee 100644 --- a/ietf/utils/test_runner.py +++ b/ietf/utils/test_runner.py @@ -797,10 +797,6 @@ def setup_test_environment(self, **kwargs): "covered": {}, "format": 1, }, - "migration": { - "present": {}, - "format": 3, - } } settings.TEMPLATES[0]['OPTIONS']['loaders'] = ('ietf.utils.test_runner.TemplateCoverageLoader',) + settings.TEMPLATES[0]['OPTIONS']['loaders'] From a2cf14003168ba780cba6aaa30b5bfc9ff6283ee Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 28 Jul 2025 13:32:16 -0300 Subject: [PATCH 14/16] test: test CoverageManager class --- ietf/utils/tests_coverage.py | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 ietf/utils/tests_coverage.py diff --git a/ietf/utils/tests_coverage.py b/ietf/utils/tests_coverage.py new file mode 100644 index 0000000000..68795994a7 --- /dev/null +++ b/ietf/utils/tests_coverage.py @@ -0,0 +1,56 @@ +# Copyright The IETF Trust 2025, All Rights Reserved +"""Tests of the coverage.py module""" + +from unittest import mock + +from django.test import override_settings + +from .coverage import CoverageManager +from .test_utils import TestCase + + +class CoverageManagerTests(TestCase): + @override_settings( + BASE_DIR="/path/to/project/ietf", + TEST_CODE_COVERAGE_EXCLUDE_FILES=["a.py"], + TEST_CODE_COVERAGE_EXCLUDE_LINES=["some-regex"], + ) + @mock.patch("ietf.utils.coverage.Coverage") + def test_coverage_manager(self, mock_coverage): + """CoverageManager managed coverage correctly in non-production mode + + Presumes we're not running tests in production mode. + """ + cm = CoverageManager() + self.assertFalse(cm.started) + + cm.start() + self.assertTrue(cm.started) + self.assertEqual(cm.checker, mock_coverage.return_value) + self.assertTrue(mock_coverage.called) + coverage_kwargs = mock_coverage.call_args.kwargs + self.assertEqual(coverage_kwargs["source"], ["/path/to/project/ietf"]) + self.assertEqual(coverage_kwargs["omit"], ["a.py"]) + self.assertTrue(isinstance(cm.checker.exclude, mock.Mock)) + assert isinstance(cm.checker.exclude, mock.Mock) # for type checker + self.assertEqual(cm.checker.exclude.call_count, 1) + cm.checker.exclude.assert_called_with("some-regex") + + @mock.patch("ietf.utils.coverage.Coverage") + def test_coverage_manager_is_defanged_in_production(self, mock_coverage): + """CoverageManager is a no-op in production mode""" + # Be careful faking settings.SERVER_MODE, but there's really no other way to + # test this. + with override_settings(SERVER_MODE="production"): + cm = CoverageManager() + cm.start() + + # Check that nothing actually happened + self.assertFalse(mock_coverage.called) + self.assertIsNone(cm.checker) + self.assertFalse(cm.started) + + # Check that other methods are guarded appropriately + cm.stop() + cm.save() + self.assertIsNone(cm.report()) From e3ac8b24a5deaa3d16675cdac6329f83fb62a620 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 28 Jul 2025 13:45:39 -0300 Subject: [PATCH 15/16] chore: exclude CustomDictReporter from coverage Setting up to test this will be complex and we'll notice other test failures/coverage weirdness if this does not behave. --- ietf/utils/coverage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ietf/utils/coverage.py b/ietf/utils/coverage.py index 5e40c1edc7..bd205ce586 100644 --- a/ietf/utils/coverage.py +++ b/ietf/utils/coverage.py @@ -46,7 +46,7 @@ def report(self, include: list[str] | None = None): return reporter.report(self.checker) -class CustomDictReporter: +class CustomDictReporter: # pragma: no cover total = Numbers() def report(self, coverage): From d6d41ededf601b6bea02ebce4b80af892fcccc7a Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 28 Jul 2025 15:44:22 -0300 Subject: [PATCH 16/16] chore: exclude coverage.py from coverage Way too meta --- ietf/settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ietf/settings.py b/ietf/settings.py index 2f7baa7a9a..e7134de55d 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -711,6 +711,7 @@ def skip_unreadable_post(record): "ietf/utils/patch.py", "ietf/utils/test_data.py", "ietf/utils/jstest.py", + "ietf/utils/coverage.py", ] # These are code line regex patterns