I'm working on a code generation module and I need to check that generated classes have the right package.
Currently, this is the best AssertJ lets me express:
Class<?> aClass;
String packageName;
assertThat(aClass.getPackageName()).isEqualTo(packageName);
I would like a more expressive assertion such as:
assertThat(aClass).hasPackage(packageName);
There could be an overload for a Package argument as well.
Fine by me.
What about
assertThat(aClass).isInPackage(packageName);
Not sure what reads better.
Your proposal is good as well, I didn't spend too much time thinking about the assertion name. Another proposal, following what I see inside AbstractClassAssert would be
hasDeclaredPackage(packageName)
@joel-costigliola what do you think would be a good name for this assertion?
Anyway, I could try and implement it if you don't mind.
Considering the existing style of AbstractClassAssert together with getPackageName(String) and getPackage(java.lang.Package)in java.lang.Class, being 100% in sync would lead to:
hasPackageName(String)hasPackage(Package)Maybe just hasPackage overloaded is enough.
@joel-costigliola could you assign this issue to me, as I started working on it, please?
Thanks @polarene!
You're welcome, I just need to carve out some time between work and family 馃槃
@polarene take your time!
@polarene can you give us an update on your progress?
We are happy to finish this one if you don't have the time (which is totally ok).
Hi @joel-costigliola I've implemented the checks, it took me some time to figure out where to modify or to add classes because there's no guide on this, so I tried to mimic nearby code. I'm missing the internal tests and then should be done. Could you just give me a few more days? I'd really like to complete this PR for the sake of contributing, then if I see I can't make it I'll hand it over to you.
@polarene take your time and please do let us know if you need anything from us.
I'm really having a hard time figuring out which kind of tests go in org.assertj.core.api.* and org.assertj.core.internal.*, since some only verify internals interaction, some verify the API using assertThat(), but they all have different styles and it's difficult to find a pattern. Do you have any advice?
You're right that there are different styles around, slowly we are trying to uniform them.
Assuming you are creating AbstractClassAssert#hasPackage() and Classes#assertHasPackage(), my suggestion would be to follow the style of hasSuperclass():
hasPackage() in org.assertj.core.api.classes which verifies the internal interactionsassertHasPackage() in org.assertj.core.internal.classes which checks the real assertion logic.Also, feel free to raise a draft PR so we can have a direct look at the code.
Cool thanks for the pointers @scordio, I've already created the first kind of test, I'm trying to write the latter now. Sure, I'll open a draft PR then so you take a look, thanks again.
I think I'm done now with the PR, please check the draft if it's okay.
Just wanted to let you know that the new assertion has been immediately integrated into a project where I'm a contributor :) https://github.com/HotelsDotCom/bull/pull/196
Most helpful comment
Just wanted to let you know that the new assertion has been immediately integrated into a project where I'm a contributor :) https://github.com/HotelsDotCom/bull/pull/196