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

AllMembersSupplier treats single and array datapoint parameter type matching differently #637

Closed
pimterry opened this issue Feb 17, 2013 · 2 comments

Comments

@pimterry
Copy link
Contributor

(This is primarily a note for me to fix this shortly, since I've come up against it in #639)

This fails, saying no valid parameters were found:

@RunWith(Theories.class)
public static class SingleDataPointTheory {
    @DataPoint
    public static Object num() {
        return 10;
    }

    @Theory
    public void theory(Integer x) {
    }
}

This meanwhile, happily passes, using the parameter from the array:

@RunWith(Theories.class)
public static class DataPointArrayTheory {
    @DataPoints
    public static Object[] num() {
        return new Object[] { 10 };
    }

    @Theory
    public void theory(Integer x) {
    }
}

In addition, this doesn't pass (while the single @DataPoint equivalent does), as it finds no parameters:

@RunWith(Theories.class)
public static class DataPointArrayTheoryWithPrimitives {
    @DataPoints
    public static int[] num() {
        return new int[] { 10 };
    }

    @Theory
    public void theory(int x) {
    }
}

This is because of two closely related problems:

  • AllMembersSupplier chooses which values to use for parameters in slightly different ways depending on where the parameter came from. For all fields and single-valued DataPoint methods, it's based on the static type information available (the method return type/field type). For multi-valued DataPoints methods, it's based on the actual type of the value in the DataPoints array (note that this differs both from the single-valued version, and the multi-valued field array version), as tested by ParameterSignature.canAcceptValue()
  • The values examined in canAcceptValue are boxed first, and ParameterSignature.canAcceptValue doesn't take into account boxing. Thus canAcceptValue(Integer) fails for int method parameters, meaning int's found in DataPoints arrays returned by these methods are never considered as potential parameters. In addition, canAcceptValue(int) will fail for Integer method parameters, but since the values are always autoboxed first this never comes up.

I'd suggest that either all fields and methods are based purely on whether the static type is assignable (e.g. Object fields are never used for Integer parameters, even if the values are Integers), or that parameters are always considered on their value (i.e. field type is irrelevant, as long as the values can be used for the given parameter).

I think the second is more intuitive myself; any datapoint that can be legitimately used for a parameter should be. Also because I've got another nearly finished implementation for Iterable datapoints that requires that behaviour, as you can't get the type of an Iterable directly, unlike with String[], so you have to always examine these values.

@dsaff
Copy link
Member

dsaff commented Feb 18, 2013

I can get behind the idea of always using runtime type info. Thanks for raising this.

dsaff pushed a commit that referenced this issue Mar 19, 2013
Fixes for #637, making theory parameters assignment more consistent across sources
@Stephan202
Copy link
Contributor

I think this issue can be closed; the pull request was merged and the release notes document the changes.

@kcooney kcooney closed this as completed Jul 11, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants