Anim: Unit tests for "Insert Needed" #116419

Christoph Lendenfeld · 2023-12-21T12:13:25+01:00

2023-12-21 12:13:25 +01:00

Add unit tests for the user preference option "Insert Needed"

Basic tests for objects and bones that check if autokeying in
combination with "Insert Needed" only

keys all location channels on the first key
keys only the modified channel on the second key

It is supposed to add only keyframes that have been affected
by the used transform operation.
E.g. translating an object will only add keys on translation keys.

The behavior of keying all property array channels first, and then
only add keys on values that have actually changed may change
in the future. Ideally it would only key actual changes to begin
with. But there is no way to do this right now.

Add unit tests for the user preference option "Insert Needed" Basic tests for objects and bones that check if autokeying in combination with "Insert Needed" only * keys all location channels on the first key * keys only the modified channel on the second key It is supposed to add only keyframes that have been affected by the used transform operation. E.g. translating an object will only add keys on translation keys. The behavior of keying all property array channels first, and then only add keys on values that have actually changed may change in the future. Ideally it would only key actual changes to begin with. But there is no way to do this right now.

Christoph Lendenfeld added the

Module

Animation & Rigging

label 2023-12-21 12:13:25 +01:00

Christoph Lendenfeld added 1 commit 2023-12-21 12:13:36 +01:00

5d123d652a unit tests

Sybren A. Stüvel requested changes 2023-12-21 12:24:06 +01:00

tests/python/bl_animation_keyframing.py

						
				@ -398,0 +420,4 @@

				            "location": (2, 1, 1)

				        }

				        for fcurve in action.fcurves:

Sybren A. Stüvel commented

2023-12-21 12:22:20 +01:00

This should check that len(expected_keys) == len(action.fcurves). Otherwise the test will succeed when there are no FCurves at all. In other words, the current test doesn't detect missing FCurves, it just detects unexpected ones.

This should check that `len(expected_keys) == len(action.fcurves)`. Otherwise the test will succeed when there are no FCurves at all. In other words, the current test doesn't detect missing FCurves, it just detects unexpected ones.

Christoph Lendenfeld commented

2023-12-21 12:36:41 +01:00

explicitly checking for 3 F-Curves self.assertEqual(len(action.fcurves), 3)

explicitly checking for 3 F-Curves `self.assertEqual(len(action.fcurves), 3)`

Sybren A. Stüvel commented

2023-12-21 15:34:56 +01:00

Yeah, my code wouldn't work well ;-) But you got the point of it!

tests/python/bl_animation_keyframing.py

						
				@ -398,0 +425,4 @@

				                raise AssertionError(f"Did not expect a key on {fcurve.data_path}")

				            self.assertEqual(expected_keys[fcurve.data_path][fcurve.array_index], len(fcurve.keyframe_points))

				        bpy.data.objects.remove(keyed_object, do_unlink=True)

Sybren A. Stüvel commented

2023-12-21 12:23:51 +01:00

This seems to be a teardown, basically to prevent this test from influencing the next one. This should be done in a teardown function (docs), so that it's ensured to be run even when the test itself fails.

This seems to be a teardown, basically to prevent this test from influencing the next one. This should be done in a teardown function ([docs](https://docs.python.org/3/library/unittest.html#unittest.TestCase.tearDown)), so that it's ensured to be run even when the test itself fails.

Christoph Lendenfeld commented

2023-12-21 12:38:09 +01:00

thanks, did not know that was a thing. Also made an override to the setUp function so it is a bit clearer which user prefs are set

I will check the other tests to see if they can be refactored with setUp and tearDown. Different PR though

thanks, did not know that was a thing. Also made an override to the `setUp` function so it is a bit clearer which user prefs are set I will check the other tests to see if they can be refactored with `setUp` and `tearDown`. Different PR though

👍 1

Sybren A. Stüvel commented

2023-12-21 15:36:09 +01:00

The bpy.data.objects.remove(keyed_object, do_unlink=True) call also should be in the teardown though, otherwise the object will stay there when the test fails.

The `bpy.data.objects.remove(keyed_object, do_unlink=True)` call also should be in the teardown though, otherwise the object will stay there when the test fails.

Christoph Lendenfeld commented

2023-12-21 16:12:11 +01:00

I don't think this is an issue because the base class AbstractKeyframingTest has the setUp function:

def setUp(self):
    super().setUp()
    bpy.ops.wm.read_homefile(use_factory_startup=True)

I don't think this is an issue because the base class `AbstractKeyframingTest` has the `setUp` function: ``` def setUp(self): super().setUp() bpy.ops.wm.read_homefile(use_factory_startup=True) ```

Sybren A. Stüvel commented

2023-12-21 16:13:23 +01:00

In that case the object removal code can be removed altogether -- either it's worth calling (and thus ensuring it gets called), or it's worth removing. When in doubt, throw it into the canal!

👍 1

Christoph Lendenfeld commented

2023-12-21 16:23:48 +01:00

threw it into the canal. Will do the same with the other tests in an upcoming refactor PR

👍 1

Christoph Lendenfeld added 1 commit 2023-12-21 12:36:07 +01:00

28c39cf08c review comments

Christoph Lendenfeld requested review from Sybren A. Stüvel 2023-12-21 12:38:19 +01:00

Sybren A. Stüvel requested changes 2023-12-21 15:36:45 +01:00

Sybren A. Stüvel left a comment

Just one last move to setup/teardown.

Christoph Lendenfeld added 1 commit 2023-12-21 16:22:56 +01:00