Closed mjustin closed 6 years ago
Hi,
Honestly, I see no big deal here. But you always can implement your own AbstractMethodComparator and use it with your DataStrategy.
Thanks, Daniil
This seems to be rather difficult to implement as it requires ordering primitive types and to be honest I cannot say, why should anyone prefer int over double, for example. On the other hand this is RandomDataProviderStrategy, so such behavior is completely acceptable. Users definitely can implement their own DataProviderStrategy, or define own AbstractMethodComparator or create custom TypeManufacturer for BigDecimal, whatever suits you the best.
Thanks, Daniil
Fair enough, and I agree that there's not necessarily a best ordering. I guess I was just thinking something like passing the array of constructors through an arbitrary yet consistent comparator, so the chosen constructor is the same regardless of the underlying VM so long as the version of Java is the same.
e.g.
Comparator<Class<?>> classComparator = Comparator
.comparing((Function<Class<?>, Boolean>) Class::isPrimitive).reversed()
.thenComparing(Class::getName);
Comparator<Constructor<?>> consistentComparator =
Comparator.comparing((Function<Constructor<?>, Integer>) Constructor::getParameterCount)
.thenComparing(Constructor::getParameterTypes, (p1, p2) -> {
for (int i = 0; i < p1.length; i++) {
int classComparison = classComparator.compare(p1[i], p2[i]);
if (classComparison != 0) {
return classComparison;
}
}
return 0;
});
I think my main issue here is that the pojoClass.getConstructors()
approach currently used feels consistent across runs when using the library. It's only when running it on different VMs on different OSes that it starts to use a different way to create the object, depending on how the VM happens to order the constructors. And the reason for this is completely opaque to users of this library.
I recently discovered that my test's BigDecimals were getting generated differently depending on whether I was running the test locally (Mac OS X 10.13.2, Java 1.8.0_161) or on a Jenkins automation server (Linux 4.1.12-94.3.6.el6uek.x86_64, Java 1.8.0_151). On the plus side, it did show me an issue I had with my tests where they wouldn't run with certain types of values. On the other hand, my tests were passing locally, but not on Jenkins.
Specifically, locally it would generate BigDecimals with integral values such as:
On Jenkins, it would generate BigDecimals with fractional decimal values:
Doing a little digging locally, I suspect this is due to the constructors being ordered differently on the two different machines when choosing the BigDecimal constructor to use (PodamFactoryImpl.instantiatePojo()). Debugging it locally I can see that it's using the BigInteger constructor. I am not sure what Jenkins is using, but based on the generated values, it clearly cannot be that.
I see two issues here. First, I suspect it would be nice if the sorting guaranteed a consistent ordering of constructors, so that objects would be created in the same manner across machines (within the same version of Java, at least). So maybe after comparing whether annotations are present and the complexity of the constructor, if there's still a tie, sort the constructors based on the fully qualified class names of their parameters. That way the ordering is guaranteed so long as the versions of Java haven't added or removed consturctors.
Secondly, for something as common as a BigDecimal, perhaps it would be best to default to some sensible hardcoded mechanism (e.g. always have a certain number of digits both before and after the decimal point, or always pick a reasonable-sized value and scale, or something).