Recently, length of test names have been getting out of hand. This is quite inconvenient for for viewing in the console, as all the lines are aligned based on the longest one. In practice this causes output line wrapping even for console window 160 characters wide.
Descriptive names are good, but a name does not have to be a whole sentence. This is the case for the worst “offenders”. Can we agree on shortening these names?
Here is a list of the longest test names. The longest one is 124, and the last on this excerpt 71 characters long.
Our SW Guide does not establish a convention for this as of now, but may be it should draw certain general lines.
Although it is a matter of tastes, I do agree that too long names do not help much, although adding a test is already better than not having it. That being said, I would discourage the use of punctuation marks, and I would use CamelCase, as customary in ITK classes, to name the tests.
The tests names with the period in the names are google tests. When you define a google test you do it like this:
TEST(TestCaseName, TestName) {
... test body ...
}
And then CMake names the tests TestCaseName.TestName. It should be possible to group the google tests by the TestCaseName which envokes all the TestNames tests.
For this case the name of the class is BSplineSmoothingOnUpdateDisplacementFieldTransformParametersAdaptor. This follows the basic conventions of itkNameOfClasssTest#.
The google tests may follow the convention: TEST(NameOfClass, some_descriptive_name_of_the_test)
GTests being TestCaseName.TestName is fine. But TestCaseName.TestCaseName_TestName provides no benefit, and reduces readability. Not to mention increase length of the name considerably. some_descriptive_name_of_the_test could be a little more condensed/shorter for the tests on this list.
Regarding CTest having a TestCaseName calling the sub test_names, the CMake documentation has the following to say:
Both commands […] register tests, and will create a separate CTest test for each Google Test test case… Note that this is in some cases less efficient, as common set-up and tear-down logic cannot be shared by multiple test cases executing in the same instance. However, it provides more fine-grained pass/fail information to CTest, which is usually considered as more beneficial. By default, the CTest test name is the same as the Google Test name (i.e. suite.testcase); see also TEST_PREFIX and TEST_SUFFIX.
CMake’s terminology differs from that in the GTest Primer which I used. However, the above recommendations is to have CTest not run the GTests in any group/batch fashion. For example ./ITKCommonGTestDriver --gtest_filter=SmartPointer\.\*
There are some reasons to want to look into running GTests in groups.
the long names generated,
there can be a large number of the individual tests
more time is spent creating the processes and many tests burden CTest or CDash
I don’t think that we should change calling the individual tests because it goes against the CMake recommendation and would require additional CMake effort. However, these are just issues to be made aware of.
First of all I’m glad we agree that descriptive names are A Good Thing. While I agree that a test name should not be longer than necessary, I think it should still be long enough to indicate clearly which specific requirement is tested. (And I think it’s usually preferable for a test function to test one specific requirement.) So I would certainly not like to have a “hard” (enforced) upper limit on the length of a test name. However, I can imagine having a suggested or preferred maximum length (which might still be exceeded, when there is a specific reason for that particular test).
A related issue: Unfortunately, it appears that GTest does not support underscores in test names. As I see now at the Googletest FAQ:
for simplicity, we just ask the users to avoid _ in TestCaseName and TestName
Although underscores in test function names just seem to work well, Googletest just does not want to support them officially. I find that very disappointing, as I find underscores helpful to write clearer test names… Anyway, I’ll submit a patch to remove those underscores from GTest test names (replacing them by CamelCase), so that you can judge for yourself…
Please also see if you can generally shorten them. And do avoid repeating e.g. ImageNeighborhoodOffsets does not need to be right of the “dot” if it is already left of the dot.
Excuse me, @dzenanz, but that would actually cause a significant loss of meaning. These three tests are not about neighborhood offsets in general. They are specifically testing the GenerateHyperrectangularImageNeighborhoodOffsets function from itkImageNeighborhoodOffsets.h.
Fortunately these test names have already been shortened in the meantime, by two merged patches:
While I agree that a test name should not be longer than necessary, I think it should still be long enough to indicate clearly which specific requirement is tested. (And I think it’s usually preferable for a test function to test one specific requirement.) So I would certainly not like to have a “hard” (enforced) upper limit on the length of a test name. However, I can imagine having a suggested or preferred maximum length (which might still be exceeded, when there is a specific reason for that particular test).
I find that very disappointing, as I find underscores helpful to write clearer test names… Anyway, I’ll submit a patch to remove those underscores from GTest test names (replacing them by CamelCase), so that you can judge for yourself…