pojo-tester icon indicating copy to clipboard operation
pojo-tester copied to clipboard

Instantiating a pojo which ctor accepts another pojo

Open gedl opened this issue 8 years ago • 19 comments

Hi,

I have the following case:

public class B()
{
   public B(String s)
  {
   if(!"accepted".equals(s) && !"hello".equals(s))
  {
   throw IllegalArgumentException("Argument must be the string 'accepted' or 'hello' ");
  }
  }
}

public class A()
{
  public A(String s, B a)
{
   [...]
}
}

The problem is when testing A, POJO-TESTER tries to instantiate B with the parameter www.pojo.pl which will trigger the parameter check on class Band make A's test error.

There is probably a way of telling POJO-TESTER to use a custom instantiator for Bthat will always pass a valid parameter to the constructor.

From what I can see, this will happen with any POJO which only ctor accepts non-trivial classes which in turn validate the parameters they accept on their ctors. I've traced the www.pojo.nl to the class StringClassInstantiator but I don't know how to override this behaviour

Am I missing something ?

gedl avatar Apr 03 '17 16:04 gedl

Hi, actually there is no way to override any Instantiator, but we can implement this as new feature. Have you tried create(clazz, paramteters, parametersTypes) method to tell pojo-tester which constructor parameters it should use? http://www.pojo.pl/writing-tests/#choose-constructor

sta-szek avatar Apr 04 '17 08:04 sta-szek

Hi @sta-szek

Thanks for replying.

Yes, that works OK if the class under test is B. I also use that method when class under test is Aand it works fine for a few tests, but not when POJO-TESTER tries to make permutations on the parameters of ``'s ctor, which means creating instances of B with "random" ctor parameters, falling. As B ctors takes strings, POJO-TESTER will feed it www.pojo.pl, which will throw the IllegalArgumentException and error the test.

gedl avatar Apr 04 '17 08:04 gedl

OK, I see. Are you willing to contribute?

BTW Try not to use default field value changer http://www.pojo.pl/writing-tests/#configure-fvc Let me know if it works.

sta-szek avatar Apr 04 '17 13:04 sta-szek

Yes, I tried (my using a with(XXX), XXXbeing a BValueChanger) but it erroed before it got called.

I'd be happy to contribute. I've read through the source code and would make a PR from a fork, but could definitely benefit from getting some pointers on where to extend it.

I'm envisioning a "with"-like method on the Assertions builder, specifying a set of Instantiators. Those instantiators would then be called in succession, specifically a method CanInstantiate(Class<?> clazz). When it returns true, it would then call a second method Class<?> Instantiate()that would contain the necessary logic to return new, valid and pseudo-random instances of the necessary class.

Where in the lifecycle of the test would I hook up the logic of selecting such an instantiator?

gedl avatar Apr 04 '17 14:04 gedl

I like the idea with with method. I thkink the best place to use custom Instantiators is the Instantiable class, and also, you probably will have to make some refactoring - we need to pass instantiators from Assertion class (similar to FieldValueChangers) to Instantiable or call it before?

It would be nice to use lambda style e.g. withInstantiator(String.class, () -> "new string"); as it would make the instantiators easier to implement, but it's up to you.

Any contributions are welcomed.

sta-szek avatar Apr 05 '17 05:04 sta-szek

Was there any progress on this? Having a problem where my POJOS are taking a ZonedDateTime but it cannot instantiate a ZoneId to construct one as it doesn't have a default constructor and is created through static factory methods.

CCob avatar Nov 11 '17 21:11 CCob

@CCob, yes, I'm working on it. The implementation is really simple but some refactor is needed. After that I have to bring my CI back to life to avoid manually deploying new version and gitbook docs.

It may take me some time but I think this can be done this week.

sta-szek avatar Nov 13 '17 17:11 sta-szek

Great. If there is anything you'd like me to test the feel free to ask. I have unit tests that are currently failing that I can run any new code base against 👍

CCob avatar Nov 13 '17 17:11 CCob

@CCob nice! You can checkout the issue-192 branch, but this should solve only ZonedDateTime problem.

I have to refactor JavaTypeInstantiator more to add all know java types - similar to those from pl.pojo.tester.internal.field package - and I see strong dependency between those classes. Maybe they should be merged to some kind of ObjectCreator / ObjectManipulator.

If you want to do some more work you can always checkout from issue-192 branch and try to add more types to JavaTypeInstantiator class.

Here is PR with my work: https://github.com/sta-szek/pojo-tester/pull/219/files

sta-szek avatar Nov 13 '17 20:11 sta-szek

Great, I'll take a look. Have you thought about exposing AbstractObjectInstantiator as a public class that users of the library can extend. That way we could add custom object instansiators when using the library.

CCob avatar Nov 14 '17 09:11 CCob

Yes, I was thinking about it and perhaps it will be very similar to http://www.pojo.pl/writing-tests/#configure-fvc

That's why I said that

they should be merged to some kind of ObjectCreator / ObjectManipulator.

sta-szek avatar Nov 14 '17 14:11 sta-szek

Yes, agreed. Perhaps the using method could have an additional overload for a new interface called FieldValueCreator or similar

CCob avatar Nov 14 '17 14:11 CCob

FieldValueCreator is as good as FieldValueManipulator as it will create and modify field value (by creating new / copy and change).

But I think that this deserves for new issue. If you are willing to do such a refactor and feature I will be happy :)

And that should wait after my PR will be merged, otherwise there will be a lot of unobvious conflicts to resolve.

sta-szek avatar Nov 14 '17 21:11 sta-szek

I agree, certainly a new issue an PR. I can also confirm issue-192 branch now passes all my ppjo tests with the new ZonedDateTime constructor argument fix 👍

Certainly dont mind contributing but it will be as and when I can as I'm sure it is for you

CCob avatar Nov 14 '17 23:11 CCob

Nice to see pojos with ZonedDateTime constructor arguments merged. What time frame do you have in mind for 0.7.6 release?

CCob avatar Nov 17 '17 21:11 CCob

I did my best this week but I don't think that it will be sooner that december.

sta-szek avatar Nov 18 '17 21:11 sta-szek

No worries. Got a 0.7.6-SNAPSHOT on a branch right now. Will wait to merge when 0.7.6 is released. Thanks.

Sent from my Samsung Galaxy smartphone.

-------- Original message -------- From: Piotr Joński [email protected] Date: 18/11/2017 9:35 pm (GMT+00:00) To: sta-szek/pojo-tester [email protected] Cc: CCob [email protected], Mention [email protected] Subject: Re: [sta-szek/pojo-tester] Instantiating a pojo which ctor accepts another pojo (#192)

I did my best this week but I don't think that it will be sooner that december.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/sta-szek/pojo-tester/issues/192#issuecomment-345472986, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AC7BkkNTma8ZBxDb96xSN-ywG9OheeS-ks5s301_gaJpZM4MxySx.

The contents of this e-mail (including my attachments) are confidential and may be legally privileged. If you are not the intended recipient of this e-mail, any disclosure, copying, distribution or use of its contents is strictly prohibited, and you should please notify the sender immediately and then delete it (including any attachments) from your system.

The contents of this e-mail (including my attachments) are confidential and may be legally privileged. If you are not the intended recipient of this e-mail, any disclosure, copying, distribution or use of its contents is strictly prohibited, and you should please notify the sender immediately and then delete it (including any attachments) from your system.

CCob avatar Nov 18 '17 22:11 CCob

@gedl please check https://github.com/sta-szek/pojo-tester/pull/222 if this satisfies issue reported by you. Thanks.

sta-szek avatar Dec 17 '17 11:12 sta-szek

I think I might be running into the same issue when I have a class with a single constructor that takes a parameter of another POJO type. In this case, the method that tests equals reports that "The equals method should return false if objects should not be equal." but the two objects are actually equal and should be equal! I also get a similar error message for the hashCode test. If I add an empty parameter-less constructor, than my test turns green.

stephenwiebe avatar Feb 09 '18 23:02 stephenwiebe