Skip to content
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

Theories: only get all data points from enum and boolean types if the… #1651

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

awturner
Copy link

…re is no @FromDataPoints annotation, or the named data points represent a non-empty set of the same type.

Fixes #1648

…re is no @FromDataPoints annotation, or the named data points represent a non-empty set of the same type.

Fixes junit-team#1648
@marcphilipp marcphilipp added this to the 4.13.1 milestone Apr 19, 2020
@awturner
Copy link
Author

Not familiar with the pull request flow. Do I now have to do something to merge this, or is that something somebody on the project will do?

@stefanbirkner
Copy link
Contributor

The JUnit team will merge it when two of us have approved the PR.
I also added a small suggestion. It would be nice if you apply it but it is not required.

@marcphilipp marcphilipp self-requested a review April 21, 2020 17:46
} else if (paramType.equals(Boolean.class) || paramType.equals(boolean.class)) {
return new BooleanSupplier().getValueSources(unassigned);

FromDataPoints fromDataPoints = unassigned.getAnnotation(FromDataPoints.class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Shouldn't this check for ParametersSuppliedBy or, alternatively, shouldn't SpecificDataPointsSupplier throw an exception if it couldn't find a matching field?

@stefanbirkner stefanbirkner dismissed their stale review April 23, 2020 15:47

I agree with @marcphilipp that it should be part of SpecificDataPointsSupplier

@awturner
Copy link
Author

Then please can you fix this yourselves? Sorry, I just don't have enough knowledge of the code base.

@marcphilipp marcphilipp changed the base branch from master to main June 21, 2020 17:05
@marcphilipp marcphilipp removed this from the 4.13.1 milestone Oct 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@FromDataPoints("misspelled") EnumType shouldn't test all enum values
4 participants