New test for the correct naming of concrete type version classes#1791
Conversation
|
Can one of the admins verify this patch? |
michaelze
force-pushed
the
features/michaelze/add-test-for-type-version-class-naming
branch
from
d910651 to
265bd71
Compare
...rc/main/java/org/eclipse/scout/rt/dataobject/testing/AbstractTypeVersionClassNamingTest.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/eclipse/scout/rt/dataobject/testing/AbstractTypeVersionClassNamingTest.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/eclipse/scout/rt/dataobject/testing/TypeVersionClassNamingTestSupport.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/eclipse/scout/rt/dataobject/testing/AbstractTypeVersionClassNamingTest.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/eclipse/scout/rt/dataobject/testing/AbstractTypeVersionClassNamingTest.java
Show resolved
Hide resolved
| String actualNamespaceIdString = matcher.group(1); | ||
| String expectedNamespaceIdString = StringUtility.uppercaseFirst(actualNamespaceIdString.toLowerCase()); | ||
|
|
||
| return !actualNamespaceIdString.equals(expectedNamespaceIdString); |
There was a problem hiding this comment.
What will we do with all the Fixture Type Versions that we have in Scout? For example DataObjectFixture_1_0_0 will be invalid. Rename them all? Or just exclude them?
There was a problem hiding this comment.
Thats a good point which also raises the question why they do have a "wrong" name in the first place.
While developing this change I found it increasingly strange, that the rule we are testing for is not enforced more strictly in the code (e.g. when constructing a NamespaceVersion object).
On the other hand: why are we selectively converting only the first letter to lower case and not the whole namespace part of the type version class name?
What are your thoughts on that?
There was a problem hiding this comment.
Not sure why we use these names for the Fixture Type Versions that don't match the naming pattern. Maybe @paolobazzi or @LukasHuser know more about this topic?
There was a problem hiding this comment.
I think those names are a (legacy) shortcut choosen to simplify tests (and using differtent namespaces we did not have to deal with duplicated type names).
Those namespace could be adjusted/fixed (+ check for duplicated type names)
michaelze
force-pushed
the
features/michaelze/add-test-for-type-version-class-naming
branch
from
265bd71 to
0d48275
Compare
...rc/main/java/org/eclipse/scout/rt/dataobject/testing/AbstractTypeVersionClassNamingTest.java
Outdated
Show resolved
Hide resolved
| String actualNamespaceIdString = matcher.group(1); | ||
| String expectedNamespaceIdString = StringUtility.uppercaseFirst(actualNamespaceIdString.toLowerCase()); | ||
|
|
||
| return !actualNamespaceIdString.equals(expectedNamespaceIdString); |
There was a problem hiding this comment.
Not sure why we use these names for the Fixture Type Versions that don't match the naming pattern. Maybe @paolobazzi or @LukasHuser know more about this topic?
michaelze
force-pushed
the
features/michaelze/add-test-for-type-version-class-naming
branch
2 times, most recently
from
3b1509f to
e517e78
Compare
When creating a new NamespaceVersion from a type version class name, the name of the class (up until the first underscore) is converted to the namespace id with the first letter being transformed to lower case (the rest of the classname is left as-is). As all namespace ids only consist of lowercase letters, this assumes that the type version class name only consists of lowercase letters (except the first one). When this assumption is not true, problems arise when trying to resolve namespaces by the created NamespaceVersion as the contained namespace id (with incorrect casing) does not match any known namespace id. The problems that can arise are hard to debug because there's no hint at the wrong casing, every class seems to be present and have the right name. This is why we chose to contribute a test case that checks for the correct naming of ITypeVersion implementations. In addition, a test was added to the AbstractDataObjectTestCompletenessTest as a reminder to add the mentioned test class. 437982
michaelze
force-pushed
the
features/michaelze/add-test-for-type-version-class-naming
branch
from
e517e78 to
90984bb
Compare
4 participants
When creating a new NamespaceVersion from a type version class name, the name of the class (up until the first underscore) is converted to the namespace id with the first letter being transformed to lower case (the rest of the classname is left as-is). As all namespace ids only consist of lowercase letters, this assumes that the type version class name only consists of lowercase letters (except the first one). When this assumption is not true, problems arise when trying to resolve namespaces by the created NamespaceVersion as the contained namespace id (with incorrect casing) does not match any known namespace id. The problems that can arise are hard to debug because there's no hint at the wrong casing, every class seems to be present and have the right name. This is why we chose to contribute a test case that checks for the correct naming of ITypeVersion implementations. In addition, a test was added to the AbstractDataObjectTestCompletenessTest as a reminder to add the mentioned test class.