Get rid of config global state, improve tests

While dicking around with the tests I noticed that you could make them fail if you ran them twice. Many hours were spent investigating, and it turns out that Overviewer's config stuff has global state that gets modified by having parsed configurations actually modify the default config values. Not good! We can fix this by having settingsDefinition return a dict of the defaults, instead of assigning it to module-level names. We can also get rid of test_all.py, because what it would do with pytest is run all tests *twice*. Instead, do the module path stuff in __init__.py. Also, instead of throwing a non-specific Exception if exmaple isn't checked out, just skip the test thank you very much. This is the weirdest rabbit hole I've ever gone down and I haven't slept in about 30 hours. I'm going to push this commit, and if it breaks anything, I'll be blissfully asleep as the unwashed masses begin to riot over exception this traceback that.
2019-07-24 17:02:39 +02:00 · 2019-07-24 17:02:39 +02:00 · 0d3d630104
parent 0bb823599d
commit 0d3d630104
7 changed files with 70 additions and 104 deletions
--- a/.travis.yml
+++ b/.travis.yml
@ -14,13 +14,14 @@ install:
  - pip install -q pillow
  - pip install -q numpy
  - pip install -q networkx
+  - pip install -q pytest
  - python3 setup.py build
 before_script:
  - git clone git://github.com/overviewer/Minecraft-Overviewer-Addons.git ~/mcoa/
  - mkdir -p ~/.minecraft/versions/${MC_VERSION}/
  - wget -N https://overviewer.org/textures/${MC_VERSION} -O ~/.minecraft/versions/${MC_VERSION}/${MC_VERSION}.jar
 script:
-  - PYTHONPATH=. python3 test/test_all.py
+  - pytest
  - python3 overviewer.py ~/mcoa/exmaple ~/test-output --rendermodes=smooth-lighting -p1
 notifications:
  email: false
--- a/overviewer_core/config_parser.py
+++ b/overviewer_core/config_parser.py
@ -33,8 +33,9 @@ class MultiWorldParser:
        # This maps setting names to their values as given in
        # settingsDefinition.py
        self._settings = {}
-        for settingname in dir(settingsDefinition):
-            setting = getattr(settingsDefinition, settingname)
+        default_conf = settingsDefinition.get_default_config()
+        for settingname in default_conf:
+            setting = default_conf[settingname]
            if not isinstance(setting, settingsValidators.Setting):
                continue

@ -79,7 +80,6 @@ class MultiWorldParser:
        # The global environment should be the rendermode module, so the config
        # file has access to those resources.
        from . import rendermodes
-
        try:
            with open(settings_file, "rb") as settings_file_handle:
                exec(compile(settings_file_handle.read(), settings_file, 'exec'),
--- a/overviewer_core/settingsDefinition.py
+++ b/overviewer_core/settingsDefinition.py
@ -61,56 +61,60 @@ import sys
 # objects with their respective validators.

 # config file.
-renders = Setting(required=True, default=OrderedDict(),
-        validator=make_dictValidator(validateStr, make_configDictValidator(
-        {
-            "world": Setting(required=True, validator=validateStr, default=None),
-            "dimension": Setting(required=True, validator=validateDimension, default="default"),
-            "title": Setting(required=True, validator=validateStr, default=None),
-            "rendermode": Setting(required=True, validator=validateRenderMode, default='normal'),
-            "northdirection": Setting(required=True, validator=validateNorthDirection, default=0),
-            "forcerender": Setting(required=False, validator=validateBool, default=None),
-            "imgformat": Setting(required=True, validator=validateImgFormat, default="png"),
-            "imgquality": Setting(required=False, validator=validateImgQuality, default=95),
-            "imglossless": Setting(required=False, validator=validateBool,
-                                   default=True),
-            "bgcolor": Setting(required=True, validator=validateBGColor, default="1a1a1a"),
-            "defaultzoom": Setting(required=True, validator=validateDefaultZoom, default=1),
-            "optimizeimg": Setting(required=True, validator=validateOptImg, default=[]),
-            "nomarkers": Setting(required=False, validator=validateBool, default=None),
-            "texturepath": Setting(required=False, validator=validateTexturePath, default=None),
-            "renderchecks": Setting(required=False, validator=validateInt, default=None),
-            "rerenderprob": Setting(required=True, validator=validateRerenderprob, default=0),
-            "crop": Setting(required=False, validator=validateCrop, default=None),
-            "changelist": Setting(required=False, validator=validateStr, default=None),
-            "markers": Setting(required=False, validator=validateMarkers, default=[]),
-            "overlay": Setting(required=False, validator=validateOverlays, default=[]),
-            "showspawn": Setting(required=False, validator=validateBool, default=True),
-            "base": Setting(required=False, validator=validateStr, default=""),
-            "poititle": Setting(required=False, validator=validateStr, default="Markers"),
-            "customwebassets": Setting(required=False, validator=validateWebAssetsPath, default=None),
-            "maxzoom": Setting(required=False, validator=validateInt, default=None),
-            "minzoom": Setting(required=False, validator=validateInt, default=0),
-            "manualpois": Setting(required=False, validator=validateManualPOIs, default=[]),
-            "showlocationmarker": Setting(required=False, validator=validateBool, default=True),
-            "center": Setting(required=False, validator=validateCoords, default=None),
-            # Remove this eventually (once people update their configs)
-            "worldname": Setting(required=False, default=None,
-                validator=error("The option 'worldname' is now called 'world'. Please update your config files")),
-        }
-        )))
+def get_default_config():
+    conf = dict()
+    conf['renders'] = Setting(required=True, default=OrderedDict(),
+            validator=make_dictValidator(validateStr, make_configDictValidator(
+            {
+                "world": Setting(required=True, validator=validateStr, default=None),
+                "dimension": Setting(required=True, validator=validateDimension, default="default"),
+                "title": Setting(required=True, validator=validateStr, default=None),
+                "rendermode": Setting(required=True, validator=validateRenderMode, default='normal'),
+                "northdirection": Setting(required=True, validator=validateNorthDirection, default=0),
+                "forcerender": Setting(required=False, validator=validateBool, default=None),
+                "imgformat": Setting(required=True, validator=validateImgFormat, default="png"),
+                "imgquality": Setting(required=False, validator=validateImgQuality, default=95),
+                "imglossless": Setting(required=False, validator=validateBool,
+                                    default=True),
+                "bgcolor": Setting(required=True, validator=validateBGColor, default="1a1a1a"),
+                "defaultzoom": Setting(required=True, validator=validateDefaultZoom, default=1),
+                "optimizeimg": Setting(required=True, validator=validateOptImg, default=[]),
+                "nomarkers": Setting(required=False, validator=validateBool, default=None),
+                "texturepath": Setting(required=False, validator=validateTexturePath, default=None),
+                "renderchecks": Setting(required=False, validator=validateInt, default=None),
+                "rerenderprob": Setting(required=True, validator=validateRerenderprob, default=0),
+                "crop": Setting(required=False, validator=validateCrop, default=None),
+                "changelist": Setting(required=False, validator=validateStr, default=None),
+                "markers": Setting(required=False, validator=validateMarkers, default=[]),
+                "overlay": Setting(required=False, validator=validateOverlays, default=[]),
+                "showspawn": Setting(required=False, validator=validateBool, default=True),
+                "base": Setting(required=False, validator=validateStr, default=""),
+                "poititle": Setting(required=False, validator=validateStr, default="Markers"),
+                "customwebassets": Setting(required=False, validator=validateWebAssetsPath, default=None),
+                "maxzoom": Setting(required=False, validator=validateInt, default=None),
+                "minzoom": Setting(required=False, validator=validateInt, default=0),
+                "manualpois": Setting(required=False, validator=validateManualPOIs, default=[]),
+                "showlocationmarker": Setting(required=False, validator=validateBool, default=True),
+                "center": Setting(required=False, validator=validateCoords, default=None),
+                # Remove this eventually (once people update their configs)
+                "worldname": Setting(required=False, default=None,
+                    validator=error("The option 'worldname' is now called 'world'. Please update your config files")),
+            }
+            )))

-# The worlds dict, mapping world names to world paths
-worlds = Setting(required=True, validator=make_dictValidator(validateStr, validateWorldPath), default=OrderedDict())
+    # The worlds dict, mapping world names to world paths
+    conf['worlds'] = Setting(required=True, validator=make_dictValidator(validateStr, validateWorldPath), default=OrderedDict())

-outputdir = Setting(required=True, validator=validateOutputDir, default=None)
+    conf['outputdir'] = Setting(required=True, validator=validateOutputDir, default=None)

-processes = Setting(required=True, validator=int, default=-1)
+    conf['processes'] = Setting(required=True, validator=int, default=-1)

-# TODO clean up this ugly in sys.argv hack
-if platform.system() == 'Windows' or not sys.stdout.isatty() or "--simple" in sys.argv:
-    obs = LoggingObserver()
-else:
-    obs = ProgressBarObserver(fd=sys.stdout)
+    # TODO clean up this ugly in sys.argv hack
+    if platform.system() == 'Windows' or not sys.stdout.isatty() or "--simple" in sys.argv:
+        obs = LoggingObserver()
+    else:
+        obs = ProgressBarObserver(fd=sys.stdout)

-observer = Setting(required=True, validator=validateObserver, default=obs)
+    conf['observer'] = Setting(required=True, validator=validateObserver, default=obs)
+
+    return conf
--- a/test/init.py
+++ b/test/init.py
@ -0,0 +1,6 @@
+# Magic spell to have us find overviewer_core
+import sys
+import os
+import os.path
+sys.path.insert(0, os.getcwd())
+sys.path.insert(0, os.path.join(os.getcwd(), os.pardir))
--- a/test/test_all.py
+++ b/test/test_all.py
@ -1,44 +0,0 @@
-#!/usr/bin/env python3
-import unittest
-
-# For convenience
-import sys
-import os
-import logging
-
-sys.path.insert(0, os.getcwd())
-sys.path.insert(0, os.path.join(os.getcwd(), os.pardir))
-
-# Import unit test cases or suites here
-from test_tileobj import TileTest
-from test_rendertileset import RendertileSetTest
-from test_settings import SettingsTest
-from test_tileset import TilesetTest
-from test_cache import TestLRU
-from test_contributors import TestContributors
-from test_cyrillic_convert import TestCyrillicConvert
-from test_playerInspect import TestPlayerInspect
-from test_regionTrimmer import TestRegionTrimmer
-from test_testRender import TestTestRender
-
-# DISABLE THIS BLOCK TO GET LOG OUTPUT FROM TILESET FOR DEBUGGING
-if 0:
-    root = logging.getLogger()
-
-    class NullHandler(logging.Handler):
-        def handle(self, record):
-            pass
-
-        def emit(self, record):
-            pass
-
-        def createLock(self):
-            self.lock = None
-    root.addHandler(NullHandler())
-else:
-    from overviewer_core import logger
-    logger.configure(logging.DEBUG, True)
-
-
-if __name__ == "__main__":
-    unittest.main()
--- a/test/test_settings.py
+++ b/test/test_settings.py
@ -63,7 +63,11 @@ class SettingsTest(unittest.TestCase):
                }),
            ]))
        self.s.set_config_item("outputdir", "/tmp/fictional/outputdir")
-        self.assertEqual(fromfile.get_validated_config(), self.s.get_validated_config())
+        first = fromfile.get_validated_config()
+        del first["observer"]
+        second = self.s.get_validated_config()
+        del second["observer"]
+        self.assertEqual(first, second)

    def test_rendermode_string(self):
        self.s.set_config_item("worlds", {
--- a/test/test_world.py
+++ b/test/test_world.py
@ -5,15 +5,10 @@ import os
 from overviewer_core import world

 class ExampleWorldTest(unittest.TestCase):
-    @classmethod
-    def setUpClass(cls):
-        # Make sure that test/data/worlds/example exists
-        # if it doesn't, then give a little 
-        if not os.path.exists("test/data/worlds/exmaple"):
-            raise Exception("test data doesn't exist.  Maybe you need to init/update your submodule?")
-
    def test_basic(self):
        "Basic test of the world constructor and regionset constructor"
+        if not os.path.exists("test/data/worlds/exmaple"):
+            raise unittest.SkipTest("test data doesn't exist.  Maybe you need to init/update your submodule?")
        w = world.World("test/data/worlds/exmaple")

        regionsets = w.get_regionsets()