Add ParameterizedTest#argumentCountValidation#4045
Add ParameterizedTest#argumentCountValidation#4045marcphilipp merged 27 commits intojunit-team:mainfrom
Conversation
This allows parameterized tests to fail when there are more arguments provided than declared by the test method. This is done in a backwards compatible way by only enabling that validation when the new `junit.jupiter.params.argumentCountValidation` is set to `strict` or `ParameterizedTest#argumentCountValidation` is set to `ArgumentCountValidationMode.STRICT`.
2b42de8 to
2e56d56
Compare
| logger.warn(() -> String.format( | ||
| "Ignored invalid configuration '%s' set via the '%s' configuration parameter.", value, key)); |
There was a problem hiding this comment.
The error message should be the same as in EnumConfigurationParameterConverter. Unfortunately, we can't (easily) reuse that here.
There was a problem hiding this comment.
Is there some way that we could make it more easily reusable across the project? Maybe I could add another PR to address that after merging this one.
For now, I basically copied the messages from EnumConfigurationParameterConverter.
Our guideline is to introduce new features as "experimental".
I think we should add a small sample to the User Guide otherwise the chance to get feedback is slim.
Documenting preconditions is mainly interesting for public API so we can omit it here since |
# Conflicts: # junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTest.java # junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestExtension.java
- add `writing-tests-parameterized-tests-argument-count-validation` section to the user guide. - cache configuration parameter in `ExtensionContext.Store` - adjust log and error messages Issue junit-team#3708
|
Thank you for all the pointers and comments on this PR @marcphilipp. I've addressed all your comments. Could you have another look? |
marcphilipp
left a comment
There was a problem hiding this comment.
Thanks for making those changes! I think we're very close now. Please also add an entry to the 5.12.0-M1 release notes.
release-notes-5.12.0-M1-junit-jupiter-new-features-and-improvements to be exact Issue junit-team#3708
Co-authored-by: Marc Philipp <mail@marcphilipp.de>
Co-authored-by: Marc Philipp <mail@marcphilipp.de>
Co-authored-by: Marc Philipp <mail@marcphilipp.de>
Co-authored-by: Marc Philipp <mail@marcphilipp.de>
65cd018 to
0bf0b05
Compare
|
@marcphilipp could you review again? I applied all suggestions and added a release note. |
Co-authored-by: Marc Philipp <mail@marcphilipp.de>
marcphilipp
left a comment
There was a problem hiding this comment.
Looks good! I've only two small suggestions. Please let me know if you have time to do those or whether I should take over.
| } | ||
|
|
||
| private ExtensionContext.Store getStore(ExtensionContext context) { | ||
| return context.getRoot().getStore(ExtensionContext.Namespace.create(getClass())); |
There was a problem hiding this comment.
Please create a constant for ExtensionContext.Namespace.create(ArgumentCountValidator.class).
| ParameterizedTest parameterizedTest = findAnnotation(// | ||
| extensionContext.getRequiredTestMethod(), ParameterizedTest.class// | ||
| ).orElseThrow(NoSuchElementException::new); |
There was a problem hiding this comment.
Instead of looking up the annotation, we should pass ParameterizedTestMethodContext to the constructor
Thanks! I'll make those changes you suggested today or tomorrow :) |
|
@marcphilipp could you merge the PR? And of course feel free to review the small change you suggested. |
|
@JonasJebing Thank you for your contribution! 👍 |
Overview
This allows parameterized tests to fail
when there are more arguments provided than declared by the test method. This is done in a backwards compatible way
by only enabling that validation when the new
junit.jupiter.params.argumentCountValidationis set tostrictorParameterizedTest#argumentCountValidationis set toArgumentCountValidationMode.STRICT.Open Questions
Should these additions be declared as experimental or stable?Should this feature be documented in the User Guide and Release Notes, given that it is declared as experimental?Should the new precondition be documented onorg.junit.jupiter.params.ParameterizedTestExtension#provideTestTemplateInvocationContexts, even though it is just an interface override and none of the existing preconditions are documented there?I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@APIannotations