Cleanup: Remove some old modifier test assets
AbandonedPublic
Actions

Authored by Jesse Yurkovich (deadpin) on May 1 2020, 10:12 AM.

Details

Reviewers

Habib Gahbiche (zazizizou)
Himanshi Kalra (calra)
Howard Trickey (howardt)

Summary

Intended only for 2.9x, this cleans up the following to reduce confusion and unused files

Removes bl_mesh_modifiers.py test (it's outdated, has been superseded by modifiers.py, and hasn't been run as part of ctest for quite some time already)
Removes the old object_modifier_array test and adds it to the modifiers.py script (easier to maintain)

Note for SVN: The following file can be removed from the test SVN trunk at the same time as this commit as long as 2.83 LTS testing targets the correct SVN tag (I think it would, but not sure):

Remove .../tests/modifier_stack/array_test.blend

Replace .../tests/modeling/modifiers.blend with this new file:

modifiers.blend13 MBDownload

Diff Detail

Repository

rB Blender

Branch

modifiertests (branched from master)

Build Status

Buildable 7833
Build 7833: arc lint + arc unit

Event Timeline

Jesse Yurkovich (deadpin) created this revision.May 1 2020, 10:12 AM

Harbormaster completed remote builds in B7833: Diff 24276.May 1 2020, 10:12 AM

Jesse Yurkovich (deadpin) requested review of this revision.May 1 2020, 10:12 AM

Jesse Yurkovich (deadpin) edited the summary of this revision. (Show Details)May 1 2020, 10:14 AM

Jesse Yurkovich (deadpin) edited the summary of this revision. (Show Details)

Thanks! I tested it and it looks good to me. Although I'm not sure about the object names. There are plans to identify tests by name and not by index such that object names won't be important anymore.

tests/python/modifiers.py
144	I personally prefer a descriptive name. After going through the comments of the task T50851 and reading the mesh name in the outliner, I realized this is probably about testing if a complex merge works? So why not just call objects `testComplexMergeArray` and `expectedComplexMergeArray`?

Naming things with words like "complex" also doesn't mean much. What happens when there's an even more complex case in the future etc. In fact I debated just removing this case since it's not very complex at all :) I think bug number means a lot more and it allows people now and in the future to more easily track down the details of the test if required.

It also follows some prior naming in this test (to reference bug reports) and in the ffmpeg and cycles tests which name tests and .blend files with bug numbers as well.

In D7590#191685, @Jesse Yurkovich (deadpin) wrote:

Naming things with words like "complex" also doesn't mean much. What happens when there's an even more complex case in the future etc. In fact I debated just removing this case since it's not very complex at all :) I think bug number means a lot more and it allows people now and in the future to more easily track down the details of the test if required.

Ok fine by me, as long as the test tackles the cause of the bug (which I think it is the case here).

It also follows some prior naming in this test (to reference bug reports) and in the ffmpeg and cycles tests which name tests and .blend files with bug numbers as well.

Cycles tests are nicely displayed in an html page so you can have a good overview of the tests without looking at names. Here it's not the case, we rely on descriptive names for the overview of (variations of) modifiers

Either way, I think the patch is useful and would rather merge it as it is than keep discuss naming conventions forever.

This revision is now accepted and ready to land.Jun 11 2020, 11:55 AM

I'll need to merge in a recent change to the .blend before this can go in. Additionally, I want to see how the recent testing GSOC project plays out as it drastically changing the formatting of the .py along the way. Will put this change on hold until later in the summer I guess.

In any case, I will need help submitting in the future as I do not have commit rights (nor do I wish to get them at this moment)

Will redo the change in a clean branch.