Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Issue: #3708 add ParameterizedTest#argumentCountValidation #4045
base: main
Are you sure you want to change the base?
Issue: #3708 add ParameterizedTest#argumentCountValidation #4045
Changes from 15 commits
2e56d56
3d40ebe
98cdf69
89a7443
5c71632
edd9f46
beb53ab
b117b69
37216f8
f87404f
14a071b
762c39a
8a84dc2
09ced82
0bf0b05
f189f07
36e7729
851a4db
a986bba
2dafffb
e70240c
3d84b9f
dd6b55a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Playing around with this made me realize that we need to defer this validation until the invocation is being executed. Otherwise, all invocations will fail rather than just the offending ones. For example, annotating the added test in
ParameterizedTestDemo
with@CsvSource({ "42, -666", "1", "2", "3" })
should result in the first invocation being reported as failed due to the new validation but all others should pass.I think the cleanest way to do this is to return another extension from
ParameterizedTestInvocationContext#getAdditionalExtensions
, sayArgumentCountValidator
, that implementsInvocationInterceptor
and overridesinterceptTestTemplateMethod
with the validation before callinginvocation.proceed()
.Please let me know whether that's enough information for you to proceed. Alternatively, I'd also be willing to take over if you'd prefer that.
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.
Yes, I also thought something wasn't quite right after playing around with ParameterizedTestDemo. Thanks for the pointers with
ParameterizedTestInvocationContext#getAdditionalExtensions
. That definitely seems doable to me.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've updated the code, I think the Windows check is failing for unrelated reasons