Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use equals() first for aggregate state check #2196

Merged
merged 3 commits into from Apr 29, 2022

Conversation

abuijze
Copy link
Member

@abuijze abuijze commented Apr 18, 2022

This avoids performing reflection on classes that may not be declared
open to the module that contains the fixtures. Typically, these classes
would implement a suitable equals method.

This change will test equality using equals first, before attempting to
check fields. If equals returns false, it will still check whether an
equals method is explicitly implemented. Only when that is not the case,
a field-to-field comparison is performed using the same approach for
each individual field.

This avoids performing reflection on classes that may not be declared
open to the module that contains the fixtures. Typically, these classes
would implement a suitable equals method.

This change will test equality using equals first, before attempting to
check fields. If equals returns false, it will still check whether an
equals method is explicitly implemented. Only when that is not the case,
a field-to-field comparison is performed using the same approach for
each individual field.
@abuijze abuijze added Type: Enhancement Use to signal an issue enhances an already existing feature of the project. Priority 2: Should High priority. Ideally, these issues are part of the release they’re assigned to. labels Apr 18, 2022
@abuijze abuijze requested a review from smcvb April 18, 2022 05:03
Instead of explicitlyUnequal(), check for null and whether either class
has an equals method through or operations. As this is proceeded by
Objects#equals, the scenario of (1) either being null or (2) either not
having equals implemented points out equals failed and thus the state
check should fail. Next to this, adjust indentation for clarity.

#2196
Instead of explicitlyUnequal(), check for null and whether either class
has an equals method through or operations. As this is proceeded by
Objects#equals, the scenario of (1) either being null or (2) either not
having equals implemented points out equals failed and thus the state
check should fail. Next to this, adjust indentation for clarity.

#2196
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

@smcvb smcvb added the Status: In Progress Use to signal this issue is actively worked on. label Apr 28, 2022
@smcvb smcvb added this to the Release 4.5.10 milestone Apr 28, 2022
smcvb added a commit that referenced this pull request Apr 28, 2022
- Check if expected is null before creating the matcher
- Expand description in case matching fails due to accessibility issues
- Specify deprecation warning
- Change complexity in assertThrows

#2196
@sonarcloud
Copy link

sonarcloud bot commented Apr 28, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

81.4% 81.4% Coverage
0.0% 0.0% Duplication

@smcvb smcvb merged commit bfe98e2 into axon-4.5.x Apr 29, 2022
@smcvb smcvb deleted the fix-avoid-reflection-in-test-fixtures branch April 29, 2022 08:06
@smcvb smcvb added Status: Resolved Use to signal that work on this issue is done. and removed Status: In Progress Use to signal this issue is actively worked on. labels Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority 2: Should High priority. Ideally, these issues are part of the release they’re assigned to. Status: Resolved Use to signal that work on this issue is done. Type: Enhancement Use to signal an issue enhances an already existing feature of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants