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
CartesianProductTestExtension from JUnit examples #321
CartesianProductTestExtension from JUnit examples #321
Changes from 12 commits
bbde484
0df7381
e8ceb12
bb0f012
edbf0fd
dd8d33a
8c04e10
c5f0b28
c18b6c2
698ac4b
0bec357
05dba57
06d45a7
15ce2f2
096f41e
25bfc90
088288c
705b70c
a9a4864
733d924
21c18d6
f82643c
2b04189
5f04e74
ae17e0f
916c0e5
b76351d
cb836ff
c94cd6f
b6daad3
118c3e1
7186f69
99003da
6ef09b6
72d01df
d377620
722abb0
f22aaae
cfd8555
0f70f8c
c8f3dc4
a802ca8
b620344
6c07302
7cd5f19
0860c8e
dec1990
6c7d1d0
c89cf05
26c0bbb
6c0be91
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.
I don't like the name
factory
, because it doesn't tell what is generated. Rename toTestSetFactoryMethod
or something like this. Also Javadoc is missing.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.
Why not using our Pioneer annotation methods? (Whole class)
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.
Bit of an overkill, don't you think? We are always going to depend on JUnit (obviously) so the way JUnit finds annotations on a test method will always be good enough (was my thinking).
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.
Then you argue to throw Pioneer Annotations out, because we can use the JUnit things. The idea was Pioneer Annotation was to encapsulate the JUnit thing with our own, improved methods. But I'll leave it to you / the others which way we use.
Same for TestKit or PioneerAssertions 🤷♂️
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.
In this specific case, we don't need to improve the method used by JUnit, so encapsulating it is unnecessary.
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.
The advantage of our annotation methods is that they make it easy to find meta annotations that are _indirectly present: (if a supertype of the element is annotated), meta-present (if an annotation that is present on the element is itself annotated), or _enclosing-present (if an enclosing type [think opposite of
@Nested
] is annotated). As I see it, only meta-present is relevant here, but that should indeed be allowed. Not sure ifAnnotationSupport.findAnnotation
does that.