summaryrefslogtreecommitdiff
path: root/gitlab/tests
diff options
context:
space:
mode:
authorJohn L. Villalovos <john@sodarock.com>2021-03-01 13:41:29 -0800
committerJohn L. Villalovos <john@sodarock.com>2021-04-24 10:12:20 -0700
commit565d5488b779de19a720d7a904c6fc14c394a4b9 (patch)
treea5db757a06c9387e8de19d70afa868a0409a25a0 /gitlab/tests
parentcfc42d246a4fc9a9afa9a676efcac0774e909aab (diff)
downloadgitlab-565d5488b779de19a720d7a904c6fc14c394a4b9.tar.gz
fix: add a check to ensure the MRO is correct
Add a check to ensure the MRO (Method Resolution Order) is correct for classes in gitlab.v4.objects when doing type-checking. An example of an incorrect definition: class ProjectPipeline(RESTObject, RefreshMixin, ObjectDeleteMixin): ^^^^^^^^^^ This should be at the end. Correct way would be: class ProjectPipeline(RefreshMixin, ObjectDeleteMixin, RESTObject): Correctly at the end ^^^^^^^^^^ Also fix classes which have the issue.
Diffstat (limited to 'gitlab/tests')
-rw-r--r--gitlab/tests/objects/test_mro.py123
1 files changed, 123 insertions, 0 deletions
diff --git a/gitlab/tests/objects/test_mro.py b/gitlab/tests/objects/test_mro.py
new file mode 100644
index 0000000..5577c7c
--- /dev/null
+++ b/gitlab/tests/objects/test_mro.py
@@ -0,0 +1,123 @@
+"""
+Ensure objects defined in gitlab.v4.objects have REST* as last item in class
+definition
+
+Original notes by John L. Villalovos
+
+An example of an incorrect definition:
+ class ProjectPipeline(RESTObject, RefreshMixin, ObjectDeleteMixin):
+ ^^^^^^^^^^ This should be at the end.
+
+Correct way would be:
+ class ProjectPipeline(RefreshMixin, ObjectDeleteMixin, RESTObject):
+ Correctly at the end ^^^^^^^^^^
+
+
+Why this is an issue:
+
+ When we do type-checking for gitlab/mixins.py we make RESTObject or
+ RESTManager the base class for the mixins
+
+ Here is how our classes look when type-checking:
+
+ class RESTObject(object):
+ def __init__(self, manager: "RESTManager", attrs: Dict[str, Any]) -> None:
+ ...
+
+ class Mixin(RESTObject):
+ ...
+
+ # Wrong ordering here
+ class Wrongv4Object(RESTObject, RefreshMixin):
+ ...
+
+ If we actually ran this in Python we would get the following error:
+ class Wrongv4Object(RESTObject, Mixin):
+ TypeError: Cannot create a consistent method resolution
+ order (MRO) for bases RESTObject, Mixin
+
+ When we are type-checking it fails to understand the class Wrongv4Object
+ and thus we can't type check it correctly.
+
+Almost all classes in gitlab/v4/objects/*py were already correct before this
+check was added.
+"""
+import inspect
+
+import pytest
+
+import gitlab.v4.objects
+
+
+def test_show_issue():
+ """Test case to demonstrate the TypeError that occurs"""
+
+ class RESTObject(object):
+ def __init__(self, manager: str, attrs: int) -> None:
+ ...
+
+ class Mixin(RESTObject):
+ ...
+
+ with pytest.raises(TypeError) as exc_info:
+ # Wrong ordering here
+ class Wrongv4Object(RESTObject, Mixin):
+ ...
+
+ # The error message in the exception should be:
+ # TypeError: Cannot create a consistent method resolution
+ # order (MRO) for bases RESTObject, Mixin
+
+ # Make sure the exception string contains "MRO"
+ assert "MRO" in exc_info.exconly()
+
+ # Correctly ordered class, no exception
+ class Correctv4Object(Mixin, RESTObject):
+ ...
+
+
+def test_mros():
+ """Ensure objects defined in gitlab.v4.objects have REST* as last item in
+ class definition.
+
+ We do this as we need to ensure the MRO (Method Resolution Order) is
+ correct.
+ """
+
+ failed_messages = []
+ for module_name, module_value in inspect.getmembers(gitlab.v4.objects):
+ if not inspect.ismodule(module_value):
+ # We only care about the modules
+ continue
+ # Iterate through all the classes in our module
+ for class_name, class_value in inspect.getmembers(module_value):
+ if not inspect.isclass(class_value):
+ continue
+
+ # Ignore imported classes from gitlab.base
+ if class_value.__module__ == "gitlab.base":
+ continue
+
+ mro = class_value.mro()
+
+ # We only check classes which have a 'gitlab.base' class in their MRO
+ has_base = False
+ for count, obj in enumerate(mro, start=1):
+ if obj.__module__ == "gitlab.base":
+ has_base = True
+ base_classname = obj.__name__
+ # print(f"\t{obj.__name__}: {obj.__module__}")
+ if has_base:
+ filename = inspect.getfile(class_value)
+ # NOTE(jlvillal): The very last item 'mro[-1]' is always going
+ # to be 'object'. That is why we are checking 'mro[-2]'.
+ if mro[-2].__module__ != "gitlab.base":
+ failed_messages.append(
+ (
+ f"class definition for {class_name!r} in file {filename!r} "
+ f"must have {base_classname!r} as the last class in the "
+ f"class definition"
+ )
+ )
+ failed_msg = "\n".join(failed_messages)
+ assert not failed_messages, failed_msg