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 Cartesian test documentation #666
Conversation
Funny story: The failing demo demonstrated something, which I think we just don't do. 😳
But that fails because of the check whether the parameter type is actually an enum: class CartesianEnumArgumentsProvider<E extends Enum<E>> implements CartesianParameterArgumentsProvider<E> {
@Override
public Stream<E> provideArguments(ExtensionContext context, Parameter parameter) {
Class<?> parameterType = parameter.getType();
if (!Enum.class.isAssignableFrom(parameterType))
throw new PreconditionViolationException(
String.format("Parameter of type %s must reference an Enum type", parameterType));
// ...
} What's up with that? |
Happens too early - that check is only supposed to happen if the annotation has no parameters. |
Do you have time to fix this? For simplicity's sake, we can do it as part of this PR. |
In Discord (❓) @Michael1993 pointed out:
That's true for the documentation of the old Cartesian test extension. Is the addition correct there as well? |
No, in the old version it always had to be static, then someone suggested that it could be non-static when we were doing the new version. |
Ok, then the documentation is correct as proposed in this PR. ✅ |
…into issue/665-cartesian-test-docs
…er#666) `@CartesianTest.MethodFactory`-annotated methods can be non-static under certain circumstances, but the demo snippets showed non-static factory methods that didn't match these and were thus wrong. This change fixes that, which required moving the factory methods from non-static inner classes into the outer class. For conistency and to make things easier in the future, the same was done for all other snippets as well. The snippets were prepended with imports, which was incoherent when the snippets turned from classes to just methods. Hence the imports were removed as well, which has the eadded benefit of not having to maintain the imports and making the snippets shorter. Closes: junit-pioneer#665 PR: junit-pioneer#666
Proposed commit message:
PR checklist
The following checklist shall help the PR's author, the reviewers and maintainers to ensure the quality of this project.
It is based on our contributors guidelines, especially the "writing code" section.
It shall help to check for completion of the listed points.
If a point does not apply to the given PR's changes, the corresponding entry can be simply marked as done.
Documentation (general)
.adoc
file in thedocs
folder, e.g.docs/report-entries.adoc
.adoc
file references demo insrc/demo/java
instead of containing code blocks as text.adoc
files)Documentation (new extension)
docs/docs-nav.yml
navigation has an entry for the new extensionpackage-info.java
contains information about the new extensionCode
Contributing
README.md
mentions the new contribution (real name optional)