-
Notifications
You must be signed in to change notification settings - Fork 38.5k
Introduce tests for MultiValueMap #25160
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
Introduce tests for MultiValueMap #25160
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the use of parameterized tests is a good idea.
I've requested a few minor changes and will review again once those have been made.
spring-core/src/test/java/org/springframework/util/MultiValueMapRelatedTests.java
Outdated
Show resolved
Hide resolved
spring-core/src/test/java/org/springframework/util/MultiValueMapRelatedTests.java
Outdated
Show resolved
Hide resolved
spring-core/src/test/java/org/springframework/util/MultiValueMapRelatedTests.java
Outdated
Show resolved
Hide resolved
@sbrannen thank you very much for your review. I will also change the name of the test to: MultiValueMapAdaptersTests to reflect current changes done by Juergen. |
456cd83
to
f8376e4
Compare
f8376e4
to
18b6d96
Compare
@sbrannen done I did not trust myself to delete
But imho they are now fully migrated with the parameterized tests |
Thanks @midumitrescu, I've enabled the tests that were disabled as the fix has been added in the meantime. |
imho improved tests for gh-25140, as solicited by @jhoeller
As promised in gh-25159, I am submitting a second suggestion for Testing the exiting code of
The suggestion was to create for this MultiValueMap same tests as for
present in
This, however, produced a very high code duplication, which I did not find very elegant.
I think the better way is to use Jupiter properly, to instantiate various types of MultiValueMap and test the contract for each of them.
Like this test code is not duplicated and the contracts are consistent with each other.
This is just a suggestion.
I am open for suggestions in case this does not fit your expectations.
Kind regards