Assertj-core: Add an assertion for checking a class package

Created on 25 Jul 2020  路  16Comments  路  Source: assertj/assertj-core

Summary

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:

Example

assertThat(aClass).hasPackage(packageName);

There could be an overload for a Package argument as well.

new feature

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

All 16 comments

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():

  • One test class for hasPackage() in org.assertj.core.api.classes which verifies the internal interactions
  • Another test class for assertHasPackage() 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

Was this page helpful?
0 / 5 - 0 ratings