[assertj] initial framework.dto #375
Conversation
|
|
||
| // null safe check | ||
| String actualSymbolicName = actual.symbolicName; | ||
| if (!java.util.Objects.deepEquals(actualSymbolicName, symbolicName)) { |
There was a problem hiding this comment.
I don't think you need a deep equals here as it's a string and not a complicated object with deep nested hierarchy. Regular equals would be fine (that's what BundleAssert does).
|
|
||
| // null safe check | ||
| String actualVersion = actual.version; | ||
| if (!java.util.Objects.deepEquals(actualVersion, version)) { |
| Iterables.instance() | ||
| .assertContains(info, actual.bundles, bundles); |
There was a problem hiding this comment.
Best to steer clear of Iterables - it is in an AssertJ internal package.
| Iterables.instance() | ||
| .assertContains(info, actual.bundles, bundles.toArray()); |
There was a problem hiding this comment.
Steer clear of Iterables.
| Iterables.instance() | ||
| .assertContainsOnly(info, actual.bundles, bundles); |
There was a problem hiding this comment.
Steer clear of Iterables.
| * Abstract base class for {@link BundleDTO} specific assertions | ||
| */ | ||
|
|
||
| public abstract class AbstractBundleDTOAssert<S extends AbstractBundleDTOAssert<S, A>, A extends BundleDTO> |
There was a problem hiding this comment.
A lot of the assertion implementation here could be cut-and-pasted from AbstractBundleAssert, because they have the same fields. I think this would give a more consistent user experience rather than rolling our own (slightly different) ones from scratch. Maybe even think of a way that they could share code.
| * @throws AssertionError - if the actual BundleDTO's state is not equal to | ||
| * the given one. | ||
| */ | ||
| public S hasState(int state) { |
There was a problem hiding this comment.
This is a bitmapped field. Having the extra bitmap functionality that AbstractBundleAssert has would make for more user-friendly usage. This is especially the case for the failure message: the code as written will report something like:
Expected:
<4>
but was:
<2>
Whereas the AbstractBundleAssert implementation gives you something more human-readable like:
Expected:
<4:RESOLVED>
but was:
<2:INSTALLED>
| * Abstract base class for {@link ServiceReferenceDTO} specific assertions | ||
| */ | ||
|
|
||
| public abstract class AbstractServiceReferenceDTOAssert<S extends AbstractServiceReferenceDTOAssert<S, A>, A extends ServiceReferenceDTO> |
There was a problem hiding this comment.
Again, try and keep this consistent with AbstractServiceReferenceAssert.
| @@ -0,0 +1,215 @@ | |||
| package org.osgi.test.assertj.servicereference; | |||
There was a problem hiding this comment.
package org.osgi.test.assertj.dto.servicereference;
| @@ -0,0 +1,459 @@ | |||
| package org.osgi.test.assertj.framework; | |||
There was a problem hiding this comment.
package org.osgi.test.assertj.framework.dto;
|
Note: @stbischof, because your PR is still marked as draft, I have not made this review very thorough. These were mostly some high-level comments to help push you in the right direction before you invest too much time. Hope it helps! |
|
This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please reopen the PR. Thank you for your contributions. |
|
This pull request has been automatically closed due to inactivity. If you are still interested in contributing this, please ensure that it is rebased against the latest |
|
Could you reopen this? |
TODO: