Changeset View
Standalone View
tests/python/modules/transform_test.py
- This file was added.
| # ##### BEGIN GPL LICENSE BLOCK ##### | |||||
| # | |||||
| # This program is free software; you can redistribute it and/or | |||||
| # modify it under the terms of the GNU General Public License | |||||
| # as published by the Free Software Foundation; either version 2 | |||||
| # of the License, or (at your option) any later version. | |||||
| # | |||||
| # This program is distributed in the hope that it will be useful, | |||||
| # but WITHOUT ANY WARRANTY; without even the implied warranty of | |||||
| # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | |||||
| # GNU General Public License for more details. | |||||
| # | |||||
| # You should have received a copy of the GNU General Public License | |||||
| # along with this program; if not, write to the Free Software Foundation, | |||||
| # Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | |||||
| # | |||||
| # ##### END GPL LICENSE BLOCK ##### | |||||
| # <pep8 compliant> | |||||
| import bpy | |||||
| import os | |||||
sybren: The function tries to find a 3D View. This should be clear from the name, and if that's not… | |||||
| def get_area(): | |||||
| for area in bpy.context.screen.areas: | |||||
| if area.type == 'VIEW_3D': | |||||
Done Inline ActionsNever, ever use a plain raise. Choose the appropriate exception type, and pass it an explanation of what's going wrong. sybren: Never, ever use a plain `raise`. Choose the appropriate exception type, and pass it an… | |||||
Done Inline ActionsOf course this comment also applies to all the other plain raise invocations. sybren: Of course this comment also applies to all the other plain `raise` invocations.
| |||||
| return area | |||||
| raise | |||||
| def get_region_window(area): | |||||
| for region in area.regions: | |||||
| if region.type == 'WINDOW': | |||||
| return region | |||||
| raise | |||||
| def get_rv3d(area): | |||||
| for space in area.spaces: | |||||
| if space.type == 'VIEW_3D': | |||||
| return space.region_3d | |||||
| raise | |||||
| class TestContext: | |||||
| __slots__ = ( | |||||
| "override_context", | |||||
Done Inline ActionsSince this class is contained in a test-specific file, and it's not a test itself, there is not really a need to have "Test" in the name. MeshCompare or ObjectCompare or something along those lines would be better. sybren: Since this class is contained in a test-specific file, and it's not a test itself, there is not… | |||||
| "funcs", | |||||
| "verbose", | |||||
| "cleanup", | |||||
Done Inline ActionsNo need to repeat what's already in the __init__(); these kind of comments tend to rot quickly. I think it's more important to describe what compare_data actually means. sybren: No need to repeat what's already in the `__init__()`; these kind of comments tend to rot… | |||||
| "failed", | |||||
| "dupli_objects", | |||||
| ) | |||||
| def __init__(self, funcs): | |||||
| context = bpy.context | |||||
| area = get_area() | |||||
| region = get_region_window(area) | |||||
| self.override_context = {'area':area, 'region':region} | |||||
| self.funcs = funcs | |||||
| self.verbose = os.environ.get("BLENDER_VERBOSE") is not None | |||||
| self.cleanup = True | |||||
| self.failed = [] | |||||
| self.dupli_objects = [] | |||||
| def default_settings_set(self): | |||||
| rv3d = get_rv3d(self.override_context['area']) | |||||
| if rv3d.view_perspective != 'CAMERA': | |||||
| bpy.ops.view3d.view_camera(self.override_context) | |||||
Done Inline ActionsDon't use single-letter names. sybren: Don't use single-letter names. | |||||
Done Inline ActionsDon't change the type associated with a name. What's wrong with a loop like for row in mat_diff: for value_diff in row:? sybren: Don't change the type associated with a name.
What's wrong with a loop like `for row in… | |||||
| scene = bpy.context.scene | |||||
| tool_settings = scene.tool_settings | |||||
| scene.transform_orientation_slots[0].type = 'GLOBAL' | |||||
| tool_settings.transform_pivot_point = 'MEDIAN_POINT' | |||||
| tool_settings.snap_elements = {'INCREMENT'} | |||||
| tool_settings.use_snap = False | |||||
| tool_settings.use_snap_grid_absolute = False | |||||
Done Inline ActionsHaving break/else/continue/break on consecutive lines makes it really hard to follow what's going on. Move this code into its own function, then you can replace the first break with a return and all the other keywords can go. sybren: Having `break/else/continue/break` on consecutive lines makes it really hard to follow what's… | |||||
| tool_settings.use_snap_translate = True | |||||
| tool_settings.use_snap_rotate = False | |||||
| tool_settings.use_snap_scale = False | |||||
| tool_settings.use_proportional_edit_objects = False | |||||
| tool_settings.proportional_edit_falloff = 'SMOOTH' | |||||
Done Inline ActionsThe fact that only mesh objects are supported, and that self.compare_data actually means self.compare_meshes should be documented. sybren: The fact that only mesh objects are supported, and that `self.compare_data` actually means… | |||||
| if bpy.context.mode != 'OBJECT': | |||||
Done Inline ActionsThis should also be its own function. sybren: This should also be its own function. | |||||
| bpy.ops.object.mode_set(mode='OBJECT') | |||||
| bpy.ops.object.select_all(action="DESELECT") | |||||
| def compare_objects(self, obj_a, obj_b, compare_matrix, compare_data): | |||||
| compare_success = True | |||||
| compare_result = '' | |||||
| if compare_matrix: | |||||
| compare_success = obj_a.matrix_world == obj_b.matrix_world | |||||
| if compare_success and compare_data: | |||||
| mesh_a = obj_a.data | |||||
| mesh_b = obj_b.data | |||||
| compare_result = mesh_a.unit_test_compare(mesh=mesh_b) | |||||
| compare_success = (compare_result == 'Same') | |||||
| if compare_success: | |||||
| if self.verbose: | |||||
| print("Success!") | |||||
| return True | |||||
| else: | |||||
| print("Test comparison result: {}".format(compare_result)) | |||||
| print("Resulting object '{}' did not match expected object '{}' from file {}". | |||||
| format(obj_a.name, obj_b.name, bpy.data.filepath)) | |||||
| return False | |||||
Done Inline ActionsNo need to do a late import here. sybren: No need to do a late import here.
| |||||
| def activate_object(self, obj): | |||||
| mode = bpy.context.mode | |||||
| if mode != 'OBJECT': | |||||
| bpy.ops.object.mode_set(mode="OBJECT") | |||||
| bpy.ops.object.select_all(action="DESELECT") | |||||
| bpy.context.view_layer.objects.active = obj | |||||
| obj.select_set(True) | |||||
| if mode == 'EDIT_MESH': | |||||
| bpy.ops.object.mode_set(mode="EDIT") | |||||
| def duplicate_object(self, obj): | |||||
| mode = bpy.context.mode | |||||
| if mode != 'OBJECT': | |||||
| bpy.ops.object.mode_set(mode="OBJECT") | |||||
| self.activate_object(obj) | |||||
| bpy.ops.object.duplicate() | |||||
| if mode == 'EDIT_MESH': | |||||
| bpy.ops.object.mode_set(mode="EDIT") | |||||
| obj_dupl = bpy.context.active_object | |||||
| self.dupli_objects.append(obj_dupl) | |||||
| return obj_dupl | |||||
| def run_test(self, index): | |||||
| if self.verbose: | |||||
| print() | |||||
| print("Test {} ({})".format(index, self.funcs[index].__name__)) | |||||
| self.default_settings_set() | |||||
| sucess = self.funcs[index](self) | |||||
| if not sucess: | |||||
| self.failed.append(index) | |||||
Done Inline ActionsIf I read the code correctly, ret can get two values: None and 0.0. These are not exactly meaningful values, and the handling of ret is not None is so far away from ret = 0.0 that it makes it hard to understand what exactly is going on. sybren: If I read the code correctly, `ret` can get two values: `None` and `0.0`. These are not exactly… | |||||
Done Inline ActionsThis method is the callback used in timers (I will move it closer, improve description and change the name to make it clear). When a timers callback returns None, it is unregistered. If the return is a float, the float value represents the minimum time for the test to be run again. The fact that the tests need to run on a timers callback makes it difficult to use with unittest module. Unfortunately this is the only way I have found to be able to use --event-simulate in tests. (This is also done in other test files). mano-wii: This method is the callback used in timers (I will move it closer, improve description and… | |||||
| def run_all_tests(self): | |||||
| for i in range(len(self.funcs)): | |||||
| self.run_test(i) | |||||
| def end(self): | |||||
| if not self.dupli_objects: | |||||
| return | |||||
| if self.cleanup: | |||||
| self.default_settings_set() | |||||
| self.activate_object(self.dupli_objects[0]) | |||||
| for obj_dupl in self.dupli_objects: | |||||
| obj_dupl.select_set(True) | |||||
| bpy.ops.object.delete() | |||||
| if self.failed: | |||||
| if self.verbose: | |||||
| tot = len(self.failed) | |||||
| print('Ran', tot, 'tests,' if tot > 1 else 'test,', self.failed, 'failed') | |||||
| raise Exception("Tests {} failed".format(self.failed)) | |||||
Done Inline ActionsWhat is this mapping for? sybren: What is this mapping for? | |||||
Done Inline ActionsBlender should use a consistent enum for "mode", no matter if it is the mode in the context or the mode of the object. I will split things. mano-wii: Blender should use a consistent enum for "mode", no matter if it is the mode in the context or… | |||||
Done Inline ActionsParentheses aren't necessary here. Same for other if-statements. sybren: Parentheses aren't necessary here. Same for other `if`-statements. | |||||
The function tries to find a 3D View. This should be clear from the name, and if that's not possible, from a docstring.