Details

Reviewers

Howard Trickey (howardt)
Brecht Van Lommel (brecht)
Campbell Barton (campbellbarton)

Maniphest Tasks

T67851: Modifiers and operators automated tests

Summary

This is a small framework for automated modifiers and operators testing. Using the provided script, tests can be generated and maintained very easily (which I plan to do if the task is approved).

The attached .blend file contains test meshes and is used by the script tests/python/modifiers.py.
It is assumed that the blend file is in blender_build/lib/tests/modeling.

modifiers.blend10 MBDownload

The blend file for operators should also be located in blender_build/lib/tests/modeling, and the operators script should be in tests/python/operators.py

operators.blend1 MBDownload

Diff Detail

Repository

rB Blender

Branch

master

Build Status

Buildable 4185
Build 4185: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Brecht Van Lommel (brecht) added inline comments.Aug 11 2019, 1:58 AM

tests/python/bevel_operator.py
39 ↗	(On Diff #17017)	Style: space after comma, no space before `]`.
tests/python/modules/mesh_test.py
122 ↗	(On Diff #17017)	Style: begin comment with capital, end with dot.
176–178 ↗	(On Diff #17017)	This can be done without operators: bpy.data.objects.remove(self.expected_object, do_unlink=True)
192 ↗	(On Diff #17017)	The test.blend should not be saved when there is no update. Version control should not show the file as modified. The way I imagined this to work is that it gives you a command to actually run the test. This is needed also to easily reproduce crashes, when it can't save a .blend. Perhaps the simplest would be to add an additional parameter to the original command line, to run just one test, like: blender modifiers.blend --python modifiers.py -- --show-test 25 Which could be constructed like: command = list(sys.argv) command.remove("--background") command.append("--") command.append("--show-test") command.append("25") Then `bevel_operator.py`, `boolean_operator` and `modifiers.py` should be simplified to just constructing a list of tests then, and leaving logic including like `sys.argv` parsing to `mesh_test.py`. But I think that's a good thing anyway. It also means you can avoid passing that long Python expressions.

Brecht Van Lommel (brecht) requested changes to this revision.Aug 12 2019, 11:19 AM

This revision now requires changes to proceed.Aug 12 2019, 11:19 AM

Habib Gahbiche (zazizizou) marked 9 inline comments as done.Aug 18 2019, 5:59 PM

Habib Gahbiche (zazizizou) added inline comments.

tests/python/bevel_operator.py
25–30 ↗	(On Diff #17017)	It doesn't work. I get the error: ImportError: cannot import name 'mesh_test' from 'modules' (unknown location)s
tests/python/modules/mesh_test.py
192 ↗	(On Diff #17017)	I think generalising this in the `MeshTest` class would make it too complex. The general case has to consider N test objects, M operators and P modifiers But tests usually either test 1 object and N operations or N objects and 1 operation So I would prefer to keep the failed tests handling on the `modifiers.py` side and not in the `MeshTest` class if that's ok with you. To deduplicate code, I introduced the helper classes `OperatorTest` to handle the typical case of N objects and 1 operator.

Added helper class OperatorTest to deduplicate code.
Changed style.

Harbormaster completed remote builds in B4481: Diff 17245.Aug 18 2019, 6:02 PM

I'm wondering if this can, eventually, be used to write a test for T61532 once that bug is fixed? There's some complexities there as the cage for the Array modifier seems broken and that most easily shows up when using either knife_tool (hard to python) or knife_project against the Array'd object while in Edit mode.

So would this testing framework allow comparison of the good/bad edit-mode cage so that just an Array modifier could be used? Or, if that's not testable, would this framework allow a second operator to run, like knife_project, to then verify after?

Brecht Van Lommel (brecht) added inline comments.Aug 21 2019, 3:05 AM

tests/python/bevel_operator.py
25–30 ↗	(On Diff #17017)	I guess it's different when running from inside Blender. Still this code can be simplified: import os import sys sys.path.append(os.path.dirname(os.path.realpath(__file__))) from modules.mesh_test import OperatorTest
tests/python/modules/mesh_test.py
192 ↗	(On Diff #17017)	I'm fine with it. We can always make it more generic later if needed.

In D5357#126610, @Jesse Yurkovich (deadpin) wrote:

I'm wondering if this can, eventually, be used to write a test for T61532 once that bug is fixed? There's some complexities there as the cage for the Array modifier seems broken and that most easily shows up when using either knife_tool (hard to python) or knife_project against the Array'd object while in Edit mode.

So would this testing framework allow comparison of the good/bad edit-mode cage so that just an Array modifier could be used? Or, if that's not testable, would this framework allow a second operator to run, like knife_project, to then verify after?

This framework is meant to be used as a regression test to ensure that future changes won't break existing features. So we need a working version for a regression test to be meaningful.
I am not familiar with the python API for the knife tool, but in principle, we can combine modifiers (with or without applying them) and operators to build a regression test. So once meaningful tests have been written, the bug you mentioned should be easy to catch by such a test.

I was actually looking for bugs that should have been caught by tests to write good tests, so thanks for mentioning T61532 here!

Updated command line output.
Updated test file modifiers.blend
Implemented first useful modifiers test.

Applying all modifiers to a simple cube is not as simple as I thought. Some modifiers can either completely delete the mesh (e.g. when no vertex group is selected) or generate too many vertices and make the mesh so dense that blender becomes significantly slow.

That being said, I think we could first merge the framework. Modifier and operator tests can be updated in a different patch.

Harbormaster completed remote builds in B4591: Diff 17505.Aug 25 2019, 9:35 PM

The implementation looks good now with minor comments.

However I can't get tests to fail. Using the attached modifiers.blend and adding/removing some modifiers in the generate_modifiers list of modifiers.py, it still always passes the tests?

Opening modifiers.blend manually or through --run_test, it's also hard to know what I'm looking at. There's a long list of objects with no clear organization. The evaluated object probably also should not have the modifiers applied when opening with --run_test, so it's easier to inspect and test.

tests/python/bevel_operator.py
169 ↗	(On Diff #17505)	Name these `--run-all-tests` and `--run-test`. Using underscores here is non-standard.
181–185 ↗	(On Diff #17505)	Is this code needed, wouldn't Python itself automatically print the traceback and exit with an error code?
tests/python/modifiers.py
32 ↗	(On Diff #17505)	This list seems unused, so can be removed? It's going to go out of date quickly.
tests/python/modules/mesh_test.py
389 ↗	(On Diff #17505)	Always print this information, even if `BLENDER_VERBOSE` is not set.
467 ↗	(On Diff #17505)	Same comment as above.

This revision now requires changes to proceed.Aug 29 2019, 3:52 PM

each test in modifiers.py now has its own collection in modifiers.blend
running test with --run-test now does not apply modifiers
updated style

Harbormaster completed remote builds in B4722: Diff 17746.Sep 1 2019, 9:20 PM

Habib Gahbiche (zazizizou) added inline comments.Sep 1 2019, 9:26 PM

tests/python/bevel_operator.py
181–184 ↗	(On Diff #17746)	I think `sys.exit(1)` is necessary for the ctest to fail. This is why the modifiers test was passing even though an exception was thrown in `main()`. Without `traceback.print_exc()` I see nothing in the console in case blender crashes (e.g. when class was instantiated with wrong number of parameters). So that's what I use this code for.

Added more modifiers tests
Updated modifiers.blend with more test meshes.

Can we merge this patch so that we benefit from the existing running tests and make it easier for others to contribute with tests as well?

Harbormaster completed remote builds in B5423: Diff 19125.Oct 19 2019, 5:04 PM

Added a few operators to demonstrate how to use the framework for operators.

Harbormaster completed remote builds in B5430: Diff 19135.Oct 20 2019, 6:21 PM

Added missing file in previous revision: operators.py

Harbormaster completed remote builds in B5473: Diff 19232.Oct 28 2019, 9:06 PM

Added more tests for operators
updated operators.blend

Harbormaster completed remote builds in B5612: Diff 19481.Nov 7 2019, 9:36 PM

cleaned up modifiers.blend for a better overview of tests
added modifiers tests for Armature and Cast modifiers

Harbormaster completed remote builds in B5621: Diff 19500.Nov 9 2019, 1:58 PM

cleaned up operators.blend: added labels and annotations to better explain the tests
added more operators tests with default parameters on primitive meshes.

Harbormaster completed remote builds in B5623: Diff 19507.Nov 10 2019, 3:35 PM

Dalai Felinto (dfelinto) added projects: BF Blender, Platforms, Builds, Tests & Devices.Nov 13 2019, 7:27 PM

added more operators tests
updated operators.blendfile

Harbormaster completed remote builds in B5706: Diff 19677.Nov 17 2019, 11:35 AM

added deform modifiers tests
updated modifiers.blend

Harbormaster completed remote builds in B5709: Diff 19684.Nov 17 2019, 8:05 PM

Habib Gahbiche (zazizizou) mentioned this in T71834: Compositor automated tests.Nov 24 2019, 12:22 AM

Habib Gahbiche (zazizizou) added a reviewer: Campbell Barton (campbellbarton).Nov 30 2019, 2:01 PM

Habib Gahbiche (zazizizou) mentioned this in T72069: Smooth Vertices called from the F3 operator search will put you in a modal without knowing [doesnt execute immediately].Dec 1 2019, 8:54 AM

Cover test case from T71971

Harbormaster completed remote builds in B5872: Diff 20002.Dec 1 2019, 10:55 PM

Habib Gahbiche (zazizizou) mentioned this in T71971: Inset with depth and offset relative has weird geometry and assert crash.Dec 1 2019, 10:57 PM

Himanshi Kalra (calra) added a subscriber: Himanshi Kalra (calra).Jan 7 2020, 3:18 PM

Brecht, are there any objections remaining to accepting this patch?

I haven't reviewed the latest version, but if you have an are ok with it then don't wait for me to commit it.

Probably adding and updating tests can be simplified more, so that you can just edit the Python scripts and running BLENDER_TEST_UPDATE then automatically takes care of creating all the necessary collections and objects. But that shouldn't block this.

I tried this just now and found that test 0 of the modifiers test is now failing. Does it work on your system? If so, it may be that one or more of the modifiers produces results that depends on, say, how pointers on a particular run are hashed and we should for now not do that test.

After fixing that, I will be willing to commit this patch.

fixed failing tests caused by a change in the offset's default value of the bevel modifier
raise AttribiteException instead of printing an error message when a modifier's property does not exist.

Harbormaster completed remote builds in B6204: Diff 20701.Jan 9 2020, 7:24 PM

In D5357#153951, @Brecht Van Lommel (brecht) wrote:

Probably adding and updating tests can be simplified more, so that you can just edit the Python scripts and running BLENDER_TEST_UPDATE then automatically takes care of creating all the necessary collections and objects. But that shouldn't block this.

Thanks for the feedback. I will look into that.

@Howard Trickey (howardt) Thanks for testing. Test 0 failed because a default parameter was changed, so the failure was expected I would say. I'm not entirely sure whether we can commit this patch during bcon3 or not but since it has no effect on Blender from a user's perspective my guess is it is safe.

Commented out test 0 because it does not seem to be reproducible.

After 2 fresh builds using make full and running the modifiers test 100 times for each build, all tests passed on my system.

From my point of view, this patch is good enough to be merged in master, i.e. blender2.83 / bcon1. By the way, I don't have commit access so it would be nice if you could commit the patch if it is accepted.

Harbormaster completed remote builds in B6219: Diff 20724.Jan 11 2020, 4:41 PM

Howard Trickey (howardt) mentioned this in rBL62346: Add test files needed by D5357 for testing modifiers and operators..Jan 13 2020, 1:14 PM

Howard Trickey (howardt) mentioned this in rB3fdc04d3ee0a: Accepting patch D5357: Modifiers and operators automated testing..Jan 13 2020, 1:16 PM

This revision is accepted, and has been committed by commit 3fdc04d3ee0ae45
.

Hans Goudey (HooglyBoogly) abandoned this revision.Dec 30 2020, 3:57 AM