|
| 1 | +""" |
| 2 | +Ensure objects defined in gitlab.v4.objects have REST* as last item in class |
| 3 | +definition |
| 4 | +
|
| 5 | +Original notes by John L. Villalovos |
| 6 | +
|
| 7 | +An example of an incorrect definition: |
| 8 | + class ProjectPipeline(RESTObject, RefreshMixin, ObjectDeleteMixin): |
| 9 | + ^^^^^^^^^^ This should be at the end. |
| 10 | +
|
| 11 | +Correct way would be: |
| 12 | + class ProjectPipeline(RefreshMixin, ObjectDeleteMixin, RESTObject): |
| 13 | + Correctly at the end ^^^^^^^^^^ |
| 14 | +
|
| 15 | +
|
| 16 | +Why this is an issue: |
| 17 | +
|
| 18 | + When we do type-checking for gitlab/mixins.py we make RESTObject or |
| 19 | + RESTManager the base class for the mixins |
| 20 | +
|
| 21 | + Here is how our classes look when type-checking: |
| 22 | +
|
| 23 | + class RESTObject(object): |
| 24 | + def __init__(self, manager: "RESTManager", attrs: Dict[str, Any]) -> None: |
| 25 | + ... |
| 26 | +
|
| 27 | + class Mixin(RESTObject): |
| 28 | + ... |
| 29 | +
|
| 30 | + # Wrong ordering here |
| 31 | + class Wrongv4Object(RESTObject, RefreshMixin): |
| 32 | + ... |
| 33 | +
|
| 34 | + If we actually ran this in Python we would get the following error: |
| 35 | + class Wrongv4Object(RESTObject, Mixin): |
| 36 | + TypeError: Cannot create a consistent method resolution |
| 37 | + order (MRO) for bases RESTObject, Mixin |
| 38 | +
|
| 39 | + When we are type-checking it fails to understand the class Wrongv4Object |
| 40 | + and thus we can't type check it correctly. |
| 41 | +
|
| 42 | +Almost all classes in gitlab/v4/objects/*py were already correct before this |
| 43 | +check was added. |
| 44 | +""" |
| 45 | +importinspect |
| 46 | + |
| 47 | +importpytest |
| 48 | + |
| 49 | +importgitlab.v4.objects |
| 50 | + |
| 51 | + |
| 52 | +deftest_show_issue(): |
| 53 | +"""Test case to demonstrate the TypeError that occurs""" |
| 54 | + |
| 55 | +classRESTObject(object): |
| 56 | +def__init__(self,manager:str,attrs:int)->None: |
| 57 | + ... |
| 58 | + |
| 59 | +classMixin(RESTObject): |
| 60 | + ... |
| 61 | + |
| 62 | +withpytest.raises(TypeError)asexc_info: |
| 63 | +# Wrong ordering here |
| 64 | +classWrongv4Object(RESTObject,Mixin): |
| 65 | + ... |
| 66 | + |
| 67 | +# The error message in the exception should be: |
| 68 | +# TypeError: Cannot create a consistent method resolution |
| 69 | +# order (MRO) for bases RESTObject, Mixin |
| 70 | + |
| 71 | +# Make sure the exception string contains "MRO" |
| 72 | +assert"MRO"inexc_info.exconly() |
| 73 | + |
| 74 | +# Correctly ordered class, no exception |
| 75 | +classCorrectv4Object(Mixin,RESTObject): |
| 76 | + ... |
| 77 | + |
| 78 | + |
| 79 | +deftest_mros(): |
| 80 | +"""Ensure objects defined in gitlab.v4.objects have REST* as last item in |
| 81 | + class definition. |
| 82 | +
|
| 83 | + We do this as we need to ensure the MRO (Method Resolution Order) is |
| 84 | + correct. |
| 85 | + """ |
| 86 | + |
| 87 | +failed_messages= [] |
| 88 | +formodule_name,module_valueininspect.getmembers(gitlab.v4.objects): |
| 89 | +ifnotinspect.ismodule(module_value): |
| 90 | +# We only care about the modules |
| 91 | +continue |
| 92 | +# Iterate through all the classes in our module |
| 93 | +forclass_name,class_valueininspect.getmembers(module_value): |
| 94 | +ifnotinspect.isclass(class_value): |
| 95 | +continue |
| 96 | + |
| 97 | +# Ignore imported classes from gitlab.base |
| 98 | +ifclass_value.__module__=="gitlab.base": |
| 99 | +continue |
| 100 | + |
| 101 | +mro=class_value.mro() |
| 102 | + |
| 103 | +# We only check classes which have a 'gitlab.base' class in their MRO |
| 104 | +has_base=False |
| 105 | +forcount,objinenumerate(mro,start=1): |
| 106 | +ifobj.__module__=="gitlab.base": |
| 107 | +has_base=True |
| 108 | +base_classname=obj.__name__ |
| 109 | +ifhas_base: |
| 110 | +filename=inspect.getfile(class_value) |
| 111 | +# NOTE(jlvillal): The very last item 'mro[-1]' is always going |
| 112 | +# to be 'object'. That is why we are checking 'mro[-2]'. |
| 113 | +ifmro[-2].__module__!="gitlab.base": |
| 114 | +failed_messages.append( |
| 115 | + ( |
| 116 | +f"class definition for{class_name!r} in file{filename!r} " |
| 117 | +f"must have{base_classname!r} as the last class in the " |
| 118 | +f"class definition" |
| 119 | + ) |
| 120 | + ) |
| 121 | +failed_msg="\n".join(failed_messages) |
| 122 | +assertnotfailed_messages,failed_msg |