chore: fix/refactor code near unneeded global declarations (#8765)

* refactor: unused vars in draft.py

* refactor: unwrap/simplify random_faker() prep

* chore: types/globals in test_runner.py

* chore: drop unneeded globals

Author: jennifer-richards

ietf/doc/templatetags/wg_menu.py

@@ -62,8 +62,6 @@
 
 @register.simple_tag
 def wg_menu(flavor):
-    global parents
-
     for p in parents:
         p.short_name = parent_short_names.get(p.acronym) or p.name
         if p.short_name.endswith(" Area"):

ietf/person/factories.py

@@ -26,20 +26,22 @@
 
 fake = faker.Factory.create()
 
-def setup():
-    global acceptable_fakers
-    # The transliteration of some Arabic and Devanagari names introduces
-    # non-alphabetic characters that don't work with the draft author
-    # extraction code, and also don't seem to match the way people with Arabic
-    # names romanize Arabic names.  Exclude those locales from name generation
-    # in order to avoid test failures.
-    locales = set( [ l for l in faker.config.AVAILABLE_LOCALES if not (l.startswith('ar_') or l.startswith('sg_') or l=='fr_QC') ] )
-    acceptable_fakers = [faker.Faker(locale) for locale in locales]
-setup()
+# The transliteration of some Arabic and Devanagari names introduces
+# non-alphabetic characters that don't work with the draft author
+# extraction code, and also don't seem to match the way people with Arabic
+# names romanize Arabic names.  Exclude those locales from name generation
+# in order to avoid test failures.
+_acceptable_fakers = [
+    faker.Faker(locale)
+    for locale in set(faker.config.AVAILABLE_LOCALES)
+    if not (locale.startswith('ar_') or locale.startswith('sg_') or locale == 'fr_QC')
+]
+
 
 def random_faker():
-    global acceptable_fakers
-    return random.sample(acceptable_fakers, 1)[0]
+    """Helper to get a random faker acceptable for User names"""
+    return random.sample(_acceptable_fakers, 1)[0]
+
 
 class UserFactory(factory.django.DjangoModelFactory):
     class Meta:

ietf/utils/draft.py

@@ -65,7 +65,6 @@
 opt_debug = False
 opt_timestamp = False
 opt_trace = False
-opt_authorinfo = False
 opt_attributes = False
 # Don't forget to add the option variable to the globals list in _main below
 
@@ -1332,8 +1331,6 @@ def getmeta(fn):
 
 # ----------------------------------------------------------------------
 def _output(docname, fields, outfile=sys.stdout):
-    global company_domain
-
     if opt_attributes:
         def outputkey(key, fields):
             field = fields[key]
@@ -1373,9 +1370,8 @@ def _printmeta(fn, outfile=sys.stdout):
 # Main
 # ----------------------------------------------------------------------
 
-company_domain = {}                     # type: Dict[str, str]
 def _main(outfile=sys.stdout):
-    global opt_debug, opt_timestamp, opt_trace, opt_authorinfo, files, company_domain, opt_attributes
+    global opt_debug, opt_timestamp, opt_trace, files, opt_attributes
     # set default values, if any
     # ----------------------------------------------------------------------
     # Option processing
@@ -1423,8 +1419,6 @@ def _main(outfile=sys.stdout):
         elif opt in ["-T", "--trace"]: # Emit trace information while working
             opt_trace = True
 
-    company_domain = {}
- 
     if not files:
         files = [ "-" ]
 

ietf/utils/patch.py

@@ -87,8 +87,7 @@ def createLock(self):
 debugmode = False
 
 def setdebug():
-  global debugmode, streamhandler
-
+  global debugmode
   debugmode = True
   loglevel = logging.DEBUG
   logformat = "%(levelname)8s %(message)s"

ietf/utils/test_runner.py

@@ -53,9 +53,10 @@
 import factory.random
 import urllib3
 import warnings
-from urllib.parse import urlencode
 
 from fnmatch import fnmatch
+from typing import Callable, Optional
+from urllib.parse import urlencode
 
 from coverage.report import Reporter
 from coverage.results import Numbers
@@ -90,11 +91,11 @@
 from mypy_boto3_s3.service_resource import Bucket
 
 
-loaded_templates = set()
-visited_urls = set()
-test_database_name = None
-old_destroy = None
-old_create = None
+loaded_templates: set[str] = set()
+visited_urls: set[str] = set()
+test_database_name: Optional[str] = None
+old_destroy: Optional[Callable] = None
+old_create: Optional[Callable] = None
 
 template_coverage_collection = None
 code_coverage_collection = None
@@ -230,10 +231,12 @@ def load_and_run_fixtures(verbosity):
             fn()
 
 def safe_create_test_db(self, verbosity, *args, **kwargs):
-    global test_database_name, old_create
+    if old_create is None:
+        raise RuntimeError("old_create has not been set, cannot proceed")
     keepdb = kwargs.get('keepdb', False)
     if not keepdb:
         print("     Creating test database...")
+    global test_database_name
     test_database_name = old_create(self, 0, *args, **kwargs)
 
     if settings.GLOBAL_TEST_FIXTURES:
@@ -243,8 +246,9 @@ def safe_create_test_db(self, verbosity, *args, **kwargs):
     return test_database_name
 
 def safe_destroy_test_db(*args, **kwargs):
+    if old_destroy is None:
+        raise RuntimeError("old_destroy has not been set, cannot proceed")
     sys.stdout.write('\n')
-    global test_database_name, old_destroy
     keepdb = kwargs.get('keepdb', False)
     if not keepdb:
         if settings.DATABASES["default"]["NAME"] != test_database_name:
@@ -358,15 +362,13 @@ class TemplateCoverageLoader(BaseLoader):
     is_usable = True
 
     def get_template(self, template_name, skip=None):
-        global template_coverage_collection, loaded_templates
-        if template_coverage_collection == True:
+        if template_coverage_collection:
             loaded_templates.add(str(template_name))
         raise TemplateDoesNotExist(template_name)
 
 def record_urls_middleware(get_response):
     def record_urls(request):
-        global url_coverage_collection, visited_urls
-        if url_coverage_collection == True:
+        if url_coverage_collection:
             visited_urls.add(request.path)
         return get_response(request)
     return record_urls
@@ -532,7 +534,6 @@ def report_test_result(self, test):
                             ( test, test_coverage*100, latest_coverage_version, master_coverage*100, ))
 
     def template_coverage_test(self):
-        global loaded_templates
         if self.runner.check_coverage:
             apps = [ app.split('.')[-1] for app in self.runner.test_apps ]
             all = get_template_paths(apps)
@@ -760,7 +761,6 @@ def __init__(
         self.show_logging = show_logging
         self.rerun = rerun
         self.test_labels = None
-        global validation_settings
         validation_settings["validate_html"] = self if validate_html else None
         validation_settings["validate_html_harder"] = self if validate_html and validate_html_harder else None
         validation_settings["show_logging"] = show_logging
@@ -783,9 +783,6 @@ def __init__(
         self.blobstoremanager = TestBlobstoreManager() if manage_blobstore else None
 
     def setup_test_environment(self, **kwargs):
-        global template_coverage_collection
-        global url_coverage_collection
-
         ietf.utils.mail.test_mode = True
         ietf.utils.mail.SMTP_ADDR['ip4'] = '127.0.0.1'
         ietf.utils.mail.SMTP_ADDR['port'] = 2025