From 0b1790ecfe1ca1d54a9d2a656324593d05347fb4 Mon Sep 17 00:00:00 2001 From: InspectorCaracal <51038201+InspectorCaracal@users.noreply.github.com> Date: Sun, 9 Oct 2022 15:33:57 -0600 Subject: [PATCH 1/5] don't suppress tracebacks on script load --- evennia/utils/containers.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/evennia/utils/containers.py b/evennia/utils/containers.py index fbce2a3299..3d14bce91b 100644 --- a/evennia/utils/containers.py +++ b/evennia/utils/containers.py @@ -208,18 +208,8 @@ class GlobalScriptContainer(Container): if self.typeclass_storage is None: self.typeclass_storage = {} for key, data in list(self.loaded_data.items()): - try: - typeclass = data.get("typeclass", settings.BASE_SCRIPT_TYPECLASS) - script_typeclass = class_from_module(typeclass) - assert issubclass(script_typeclass, _BASE_SCRIPT_TYPECLASS) - self.typeclass_storage[key] = script_typeclass - except Exception: - logger.log_trace( - f"GlobalScriptContainer could not start import global script {key}. " - "It will be removed (skipped)." - ) - # Let's remove this key/value. We want to let other scripts load. - self.loaded_data.pop(key) + typeclass = data.get("typeclass", settings.BASE_SCRIPT_TYPECLASS) + self.typeclass_storage[key] = class_from_module(typeclass, fallback=settings.BASE_SCRIPT_TYPECLASS) def get(self, key, default=None): """ From 2e9971dc8299816ff06b34e9f18afa3abaca03fe Mon Sep 17 00:00:00 2001 From: InspectorCaracal Date: Sun, 9 Oct 2022 18:36:17 -0600 Subject: [PATCH 2/5] clean-up and test updates --- evennia/utils/containers.py | 6 -- evennia/utils/tests/test_containers.py | 101 ++++++++++++------------- 2 files changed, 47 insertions(+), 60 deletions(-) diff --git a/evennia/utils/containers.py b/evennia/utils/containers.py index 3d14bce91b..d0ed248dd1 100644 --- a/evennia/utils/containers.py +++ b/evennia/utils/containers.py @@ -19,8 +19,6 @@ from evennia.utils import logger from evennia.utils.utils import callables_from_module, class_from_module SCRIPTDB = None -_BASE_SCRIPT_TYPECLASS = None - class Container: """ @@ -201,10 +199,6 @@ class GlobalScriptContainer(Container): initialized. """ - global _BASE_SCRIPT_TYPECLASS - if not _BASE_SCRIPT_TYPECLASS: - _BASE_SCRIPT_TYPECLASS = class_from_module(settings.BASE_SCRIPT_TYPECLASS) - if self.typeclass_storage is None: self.typeclass_storage = {} for key, data in list(self.loaded_data.items()): diff --git a/evennia/utils/tests/test_containers.py b/evennia/utils/tests/test_containers.py index 0f46ec1bca..6589d98ea2 100644 --- a/evennia/utils/tests/test_containers.py +++ b/evennia/utils/tests/test_containers.py @@ -4,78 +4,71 @@ from evennia.utils import containers from django.conf import settings from django.test import override_settings from evennia.utils.utils import class_from_module +from evennia import DefaultScript -_BASE_SCRIPT_TYPECLASS = class_from_module(settings.BASE_SCRIPT_TYPECLASS) +_BASE_TYPECLASS = class_from_module(settings.BASE_SCRIPT_TYPECLASS) -class GoodScript(_BASE_SCRIPT_TYPECLASS): - pass +class GoodScript(DefaultScript): + pass -class BadScript: - """Not subclass of _BASE_SCRIPT_TYPECLASS,""" - pass - -class WorseScript(_BASE_SCRIPT_TYPECLASS): - """objects will fail upon call""" - @property - def objects(self): - from evennia import module_that_doesnt_exist +class BrokenScript(DefaultScript): + """objects will fail upon call""" + @property + def objects(self): + from evennia import module_that_doesnt_exist class TestGlobalScriptContainer(unittest.TestCase): - def test_init_with_no_scripts(self): - gsc = containers.GlobalScriptContainer() + def test_init_with_no_scripts(self): + gsc = containers.GlobalScriptContainer() - self.assertEqual(len(gsc.loaded_data), 0) + self.assertEqual(len(gsc.loaded_data), 0) - @override_settings(GLOBAL_SCRIPTS={'script_name': {}}) - def test_init_with_typeclassless_script(self): + @override_settings(GLOBAL_SCRIPTS={}) + def test_start_with_no_scripts(self): + gsc = containers.GlobalScriptContainer() - gsc = containers.GlobalScriptContainer() + gsc.start() - self.assertEqual(len(gsc.loaded_data), 1) - self.assertIn('script_name', gsc.loaded_data) + self.assertEqual(len(gsc.typeclass_storage), 0) - def test_start_with_no_scripts(self): - gsc = containers.GlobalScriptContainer() + @override_settings(GLOBAL_SCRIPTS={'script_name': {}}) + def test_start_with_typeclassless_script(self): + """No specified typeclass should fallback to base""" + gsc = containers.GlobalScriptContainer() - gsc.start() + gsc.start() - self.assertEqual(len(gsc.typeclass_storage), 0) + self.assertEqual(len(gsc.typeclass_storage), 1) + self.assertIn('script_name', gsc.typeclass_storage) + self.assertEqual(gsc.typeclass_storage['script_name'], _BASE_TYPECLASS) - @override_settings(GLOBAL_SCRIPTS={'script_name': {}}) - def test_start_with_typeclassless_script_defaults_to_base(self): - gsc = containers.GlobalScriptContainer() + @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.NoScript'}}) + def test_start_with_nonexistent_script(self): + """Missing script class should fall back to base""" + gsc = containers.GlobalScriptContainer() - gsc.start() + gsc.start() - self.assertEqual(len(gsc.typeclass_storage), 1) - self.assertIn('script_name', gsc.typeclass_storage) - self.assertEqual(gsc.typeclass_storage['script_name'], _BASE_SCRIPT_TYPECLASS) + self.assertEqual(len(gsc.typeclass_storage), 1) + self.assertIn('script_name', gsc.typeclass_storage) + self.assertEqual(gsc.typeclass_storage['script_name'], _BASE_TYPECLASS) - @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.GoodScript'}}) - def test_start_with_typeclassed_script_loads_it(self): - gsc = containers.GlobalScriptContainer() + @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.GoodScript'}}) + def test_start_with_valid_script(self): + gsc = containers.GlobalScriptContainer() - gsc.start() + gsc.start() - self.assertEqual(len(gsc.typeclass_storage), 1) - self.assertIn('script_name', gsc.typeclass_storage) - self.assertEqual(gsc.typeclass_storage['script_name'], GoodScript) + self.assertEqual(len(gsc.typeclass_storage), 1) + self.assertIn('script_name', gsc.typeclass_storage) + self.assertEqual(gsc.typeclass_storage['script_name'], GoodScript) - @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.BadScript'}}) - def test_start_with_bad_typeclassed_script_skips_it(self): - gsc = containers.GlobalScriptContainer() + @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.BrokenScript'}}) + def test_start_with_broken_script(self): + """Broken script module should traceback""" + gsc = containers.GlobalScriptContainer() - gsc.start() - - self.assertEqual(len(gsc.typeclass_storage), 0) - self.assertNotIn('script_name', gsc.typeclass_storage) - - @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.WorstScript'}}) - def test_start_with_worst_typeclassed_script_skips_it(self): - gsc = containers.GlobalScriptContainer() - - gsc.start() - - self.assertEqual(len(gsc.typeclass_storage), 0) - self.assertNotIn('script_name', gsc.typeclass_storage) + with self.assertRaises(Exception) as cm: + gsc.start() + self.fail("An exception was expected but it didn't occur.") From 77eccd3033dc62ac19d1d5b3485e9bf87e349d37 Mon Sep 17 00:00:00 2001 From: InspectorCaracal Date: Sun, 9 Oct 2022 20:15:23 -0600 Subject: [PATCH 3/5] add test for un-startable script --- evennia/utils/tests/test_containers.py | 96 +++++++++++++++----------- 1 file changed, 54 insertions(+), 42 deletions(-) diff --git a/evennia/utils/tests/test_containers.py b/evennia/utils/tests/test_containers.py index 6589d98ea2..4352a67eb6 100644 --- a/evennia/utils/tests/test_containers.py +++ b/evennia/utils/tests/test_containers.py @@ -11,64 +11,76 @@ _BASE_TYPECLASS = class_from_module(settings.BASE_SCRIPT_TYPECLASS) class GoodScript(DefaultScript): pass +class InvalidScript: + pass + class BrokenScript(DefaultScript): - """objects will fail upon call""" - @property - def objects(self): - from evennia import module_that_doesnt_exist + """objects will fail upon call""" + @property + def objects(self): + from evennia import module_that_doesnt_exist class TestGlobalScriptContainer(unittest.TestCase): - def test_init_with_no_scripts(self): - gsc = containers.GlobalScriptContainer() + def test_init_with_no_scripts(self): + gsc = containers.GlobalScriptContainer() - self.assertEqual(len(gsc.loaded_data), 0) + self.assertEqual(len(gsc.loaded_data), 0) - @override_settings(GLOBAL_SCRIPTS={}) - def test_start_with_no_scripts(self): - gsc = containers.GlobalScriptContainer() + @override_settings(GLOBAL_SCRIPTS={}) + def test_start_with_no_scripts(self): + gsc = containers.GlobalScriptContainer() - gsc.start() + gsc.start() - self.assertEqual(len(gsc.typeclass_storage), 0) + self.assertEqual(len(gsc.typeclass_storage), 0) - @override_settings(GLOBAL_SCRIPTS={'script_name': {}}) - def test_start_with_typeclassless_script(self): - """No specified typeclass should fallback to base""" - gsc = containers.GlobalScriptContainer() + @override_settings(GLOBAL_SCRIPTS={'script_name': {}}) + def test_start_with_typeclassless_script(self): + """No specified typeclass should fallback to base""" + gsc = containers.GlobalScriptContainer() - gsc.start() + gsc.start() - self.assertEqual(len(gsc.typeclass_storage), 1) - self.assertIn('script_name', gsc.typeclass_storage) - self.assertEqual(gsc.typeclass_storage['script_name'], _BASE_TYPECLASS) + self.assertEqual(len(gsc.typeclass_storage), 1) + self.assertIn('script_name', gsc.typeclass_storage) + self.assertEqual(gsc.typeclass_storage['script_name'], _BASE_TYPECLASS) - @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.NoScript'}}) - def test_start_with_nonexistent_script(self): - """Missing script class should fall back to base""" - gsc = containers.GlobalScriptContainer() + @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.NoScript'}}) + def test_start_with_nonexistent_script(self): + """Missing script class should fall back to base""" + gsc = containers.GlobalScriptContainer() - gsc.start() + gsc.start() - self.assertEqual(len(gsc.typeclass_storage), 1) - self.assertIn('script_name', gsc.typeclass_storage) - self.assertEqual(gsc.typeclass_storage['script_name'], _BASE_TYPECLASS) + self.assertEqual(len(gsc.typeclass_storage), 1) + self.assertIn('script_name', gsc.typeclass_storage) + self.assertEqual(gsc.typeclass_storage['script_name'], _BASE_TYPECLASS) - @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.GoodScript'}}) - def test_start_with_valid_script(self): - gsc = containers.GlobalScriptContainer() + @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.GoodScript'}}) + def test_start_with_valid_script(self): + gsc = containers.GlobalScriptContainer() - gsc.start() + gsc.start() - self.assertEqual(len(gsc.typeclass_storage), 1) - self.assertIn('script_name', gsc.typeclass_storage) - self.assertEqual(gsc.typeclass_storage['script_name'], GoodScript) + self.assertEqual(len(gsc.typeclass_storage), 1) + self.assertIn('script_name', gsc.typeclass_storage) + self.assertEqual(gsc.typeclass_storage['script_name'], GoodScript) - @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.BrokenScript'}}) - def test_start_with_broken_script(self): - """Broken script module should traceback""" - gsc = containers.GlobalScriptContainer() + @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.InvalidScript'}}) + def test_start_with_invalid_script(self): + """Script class doesn't implement required start methods""" + gsc = containers.GlobalScriptContainer() - with self.assertRaises(Exception) as cm: - gsc.start() - self.fail("An exception was expected but it didn't occur.") + with self.assertRaises(Exception) as cm: + gsc.start() + self.fail("An exception was expected but it didn't occur.") + + @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.BrokenScript'}}) + def test_start_with_broken_script(self): + """Un-importable script should traceback""" + gsc = containers.GlobalScriptContainer() + + with self.assertRaises(Exception) as cm: + gsc.start() + self.fail("An exception was expected but it didn't occur.") From 1ce44db28353f24cd0461e86ada341f7047147f1 Mon Sep 17 00:00:00 2001 From: InspectorCaracal Date: Fri, 28 Oct 2022 09:28:44 -0600 Subject: [PATCH 4/5] test exceptions more precisely --- evennia/utils/tests/data/broken_script.py | 4 ++++ evennia/utils/tests/test_containers.py | 24 ++++++++++------------- 2 files changed, 14 insertions(+), 14 deletions(-) create mode 100644 evennia/utils/tests/data/broken_script.py diff --git a/evennia/utils/tests/data/broken_script.py b/evennia/utils/tests/data/broken_script.py new file mode 100644 index 0000000000..0484406671 --- /dev/null +++ b/evennia/utils/tests/data/broken_script.py @@ -0,0 +1,4 @@ +from evennia import nonexistent_module, DefaultScript + +class BrokenScript(DefaultScript): + pass diff --git a/evennia/utils/tests/test_containers.py b/evennia/utils/tests/test_containers.py index 4352a67eb6..91cba86179 100644 --- a/evennia/utils/tests/test_containers.py +++ b/evennia/utils/tests/test_containers.py @@ -14,12 +14,6 @@ class GoodScript(DefaultScript): class InvalidScript: pass -class BrokenScript(DefaultScript): - """objects will fail upon call""" - @property - def objects(self): - from evennia import module_that_doesnt_exist - class TestGlobalScriptContainer(unittest.TestCase): def test_init_with_no_scripts(self): @@ -69,18 +63,20 @@ class TestGlobalScriptContainer(unittest.TestCase): @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.InvalidScript'}}) def test_start_with_invalid_script(self): - """Script class doesn't implement required start methods""" + """Script class doesn't implement required methods methods""" gsc = containers.GlobalScriptContainer() - with self.assertRaises(Exception) as cm: - gsc.start() - self.fail("An exception was expected but it didn't occur.") + with self.assertRaises(AttributeError) as err: + gsc.start() + # check for general attribute failure on the invalid class to preserve against future code-rder changes + self.assertTrue(str(err.exception).startswith("type object 'InvalidScript' has no attribute"), err.exception) - @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.test_containers.BrokenScript'}}) + @override_settings(GLOBAL_SCRIPTS={'script_name': {'typeclass': 'evennia.utils.tests.data.broken_script.BrokenScript'}}) def test_start_with_broken_script(self): """Un-importable script should traceback""" gsc = containers.GlobalScriptContainer() - with self.assertRaises(Exception) as cm: - gsc.start() - self.fail("An exception was expected but it didn't occur.") + with self.assertRaises(Exception) as err: + gsc.start() + # exception raised by imported module + self.assertTrue(str(err.exception).startswith("cannot import name 'nonexistent_module' from 'evennia'"), err.exception) From d40dfefc4499732e3765d5c1681a9306bfa5bf8a Mon Sep 17 00:00:00 2001 From: InspectorCaracal Date: Fri, 28 Oct 2022 09:44:11 -0600 Subject: [PATCH 5/5] add docstring to broken script module --- evennia/utils/tests/data/broken_script.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/evennia/utils/tests/data/broken_script.py b/evennia/utils/tests/data/broken_script.py index 0484406671..76d3e7a4c7 100644 --- a/evennia/utils/tests/data/broken_script.py +++ b/evennia/utils/tests/data/broken_script.py @@ -1,3 +1,9 @@ +""" +Defines a script module with a broken import, to catch the specific error case +in loading global scripts where the module can be parsed but has broken +dependencies. +""" + from evennia import nonexistent_module, DefaultScript class BrokenScript(DefaultScript):