Details

Keep style in blend file consistent: collections are named after operators, not test cases. Look at other collection names for more examples.
If you think a test is failing because of a bug, please add task number to todo comment. Ideally, also add comment in bug report to activate the test once fixed, if that hasn't been done already.

tests/python/operators.py
241–245	This is not really a mesh operator that changes the mesh it operates on, so it won't be part of this framework. So I think it's better to remove this from here.
335	is there a bug report for this?

This revision now requires changes to proceed.Apr 11 2021, 4:51 PM

Add test cases for T87259, addressed comments by Habib
Merge branch 'master' into add_operator_tests
Minor fixes

Harbormaster completed remote builds in B14048: Diff 36144.Apr 13 2021, 8:21 PM

Himanshi Kalra (calra) edited the summary of this revision. (Show Details)Apr 13 2021, 8:22 PM

Keep style in blend file consistent: collections are named after operators, not test cases. Look at other collection names for more examples.

Noted, thanks! I did it unconsciously, have updated the blend file.

If you think a test is failing because of a bug, please add task number to todo comment. Ideally, also add comment in bug report to activate the test once fixed, if that hasn't been done already.

Yes, that was done, added the bug number in the comments in the code as well.

tests/python/operators.py
335	No, as discussed on blender chat. I have added the normal case for vertex connect path.

In D10893#279202, @Habib Gahbiche (zazizizou) wrote:

Look at other collection names for more examples.

I should have been more explicit here. You should keep collections (as well as the position of the objects) ordered alphabetically just like the others. We do this to make adding tests and refactoring easier.

Also, remember to keep T84999 up to date, there seems to be an outdated todo there? And since we agreed add primitive operators won't be covered you could remove them from the list as well.

The rest looks fine to me.

This revision now requires changes to proceed.Apr 19 2021, 8:07 PM

We do this to make adding tests and refactoring easier.

Could you explain more on this? How does that help?

I understand the alphabetical ordering in the Collections, but by objects do you mean, to move the existing objects and make space for new objects in between to keep them sorted?
Given a long list objects, wouldn't it be easier to know new test objects are added towards the end?

Edit: The operator blend file has not been updated yet, waiting for clarification.

Himanshi Kalra (calra) retitled this revision from Added Operator tests: unsubdivide, mesh primitives, mark seam to Added Operator tests: unsubdivide, shading, vertex connect and mark seam.Apr 19 2021, 8:32 PM

Himanshi Kalra (calra) edited the summary of this revision. (Show Details)

In D10893#281215, @Himanshi Kalra (calra) wrote:

We do this to make adding tests and refactoring easier.

Could you explain more on this? How does that help?

I understand the alphabetical ordering in the Collections, but by objects do you mean, to move the existing objects and make space for new objects in between to keep them sorted?

It's mostly a matter of style to keep the file organized. Same reason we have style for code.

Given a long list objects, wouldn't it be easier to know new test objects are added towards the end?

I prefer an organized file with sorted objects over a big mess.

Himanshi Kalra (calra) edited the summary of this revision. (Show Details)Apr 22 2021, 2:38 PM

Updated the blend file with alphabetical ordering in 3D space as well as the Outliner.

Given a long list objects, wouldn't it be easier to know new test objects are added towards the end?

I prefer an organized file with sorted objects over a big mess.

This was discussed again on blender chat for more clarity, this meant having 2 separate designs

Sort by alphabetical
Sort by last modified

could lead to a "mess" which could potentially confuse new contributors which one to follow.
So, the conclusion is the 1. design i.e. Sort by Alphabetical will be followed for this file.