Skip to content

Add TestUtils.propertyValue() based on generics#3305

Merged
cppwfs merged 4 commits intospring-projects:mainfrom
artembilan:TestUtils.propertyValue
Jan 23, 2026
Merged

Add TestUtils.propertyValue() based on generics#3305
cppwfs merged 4 commits intospring-projects:mainfrom
artembilan:TestUtils.propertyValue

Conversation

@artembilan
Copy link
Copy Markdown
Member

  • Deprecate TestUtils.getPropertyValue() based on a Class param
  • Optimize TestUtils.getPropertyValue() to not create extra DirectFieldAccessor if we are at the end of the nested path
  • Refactor project code to use a new TestUtils.propertyValue()
  • Remove unnecessary now @SuppressWarnings("unchecked") from those tests
  • Some other code clean up in affected test classes

* Deprecate `TestUtils.getPropertyValue()` based on a `Class` param
* Optimize `TestUtils.getPropertyValue()` to not create extra `DirectFieldAccessor`
if we are at the end of the nested path
* Refactor project code to use a new `TestUtils.propertyValue()`
* Remove unnecessary now `@SuppressWarnings("unchecked")` from those tests
* Some other code clean up in affected test classes
@artembilan artembilan added this to the 4.1.0-M2 milestone Jan 21, 2026
@artembilan artembilan requested a review from cppwfs January 21, 2026 20:14
@artembilan
Copy link
Copy Markdown
Member Author

A lot of changes in this PR are about migrating from deprecated API.
Thanks

@artembilan
Copy link
Copy Markdown
Member Author

Now, when I look into this from PR perspective, it feels like we don't need that getPropertyValue() for Object at all.
We need to deprecated it too to avoid a paradox of choice.
It doesn't look like hard to do that, but that would be much more changes.
I can proceed with that after your review, though.

Copy link
Copy Markdown
Contributor

@cppwfs cppwfs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice update to TestUtils! Made the test code easier to read. Great cleanup work!

Turned out we don't need a new method with generics.
And we don't need an extra argument to resolve the type.
There is just enough to have a simple cast to generic argument.
In the end we are talking only about tests.

* Remove redundant (just created `TestUtils.propertyValue()`)
* Change `TestUtils.getPropertyValue()` signature for generic argument
* Refactor affected tests

With a new style we don't have a paradox of choice for this or that method.
The generic argument on the method is enough to cover all use-cases,
even if we have to add `<Object>` to satisfy `assertThat()` expectations.
We don't need an extra `Class` argument to verify value compatibility.
The simple cast to generic type is enough got tests environment.
This is slightly breaking change in the API, but that is only for tests anyway
@artembilan
Copy link
Copy Markdown
Member Author

OK. I've changed my mind and now it is not a new propertyValue(), but change to generic argument for existing Object getPropertyValue().
We really don't need extra type to check: the cast to generic argument is enough for Java to fail and gives us a good ClassCastException (if any) for test report.
This is slightly a breaking change, but no one make you to use it like we do in many tests assertThat(TestUtils.getPropertyValue()).
Extract the variable and then assert. Or like I did in our tests : <Object>.
Either way I not fully afraid of such a breaking change since it should effect maximum some tests.
Plus this is a feature of this new 4.1 version.

Thanks

Copy link
Copy Markdown
Contributor

@cppwfs cppwfs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Great work here.

@cppwfs cppwfs merged commit f0102ce into spring-projects:main Jan 23, 2026
3 checks passed
@artembilan artembilan deleted the TestUtils.propertyValue branch February 25, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants