Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error message for requiredType mismatch in NamespacedHierarchicalStore #3601

Merged
merged 2 commits into from
Dec 15, 2023

Conversation

leonard84
Copy link
Collaborator

Overview

Prior to this commit, the error message just contained the key and the requiredType, but gave no indication what was actually present. Now, the error message also includes the type of the actual value as well as its toString() representation.


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

Prior to this commit, the error message just contained the key and
the requiredType, but gave no indication what was actually present.
Now, the error message also includes the type of the actual value
as well as its toString() representation.
@marcphilipp marcphilipp merged commit c501832 into main Dec 15, 2023
12 checks passed
@marcphilipp marcphilipp deleted the leo/improve-required-type-error-message branch December 15, 2023 08:51
@marcphilipp
Copy link
Member

@leonard84 Thanks! 👍

@@ -252,7 +252,8 @@ private <T> T castToRequiredType(Object key, Object value, Class<T> requiredType
}
// else
throw new NamespacedHierarchicalStoreException(
String.format("Object stored under key [%s] is not of required type [%s]", key, requiredType.getName()));
String.format("Object stored under key [%s] is not of required type [%s], but was [%s]: %s", key,
requiredType.getName(), value.getClass(), value));
Copy link
Member

Choose a reason for hiding this comment

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

For future reference, please note that we typically invoke Class#getName when including types in messages, analogous to requireType.getName() on the same line.

I'll polish that on main since this has already been merged.

Thanks for the PR! 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense, I overlooked that it included the class prefix.

@sbrannen sbrannen changed the title Improve error message for Store requiredType mismatch Improve error message for requiredType mismatch in NamespacedHierarchicalStore Dec 15, 2023
sbrannen added a commit that referenced this pull request Dec 15, 2023
This pull request was closed.
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.

3 participants