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

Fix(Issue no. 1330) for Relaxed Mocking Value When Property is Nested Value Class #1331

Conversation

kpadhiamex
Copy link
Contributor

@kpadhiamex kpadhiamex commented Dec 19, 2024

Fix for the Issue: #1330

Fixed for given nested value classes when relaxed mocking is enabled then value successfully verified.
Added couple of test(s) so that it will be verified in every PR.

Root Cause
Because of this PR's: https://github.com/mockk/mockk/pull/1314/files change in the below code in the else part, this issue was broken. Instead of returning this in the else block which returns the value class, instead check if there is any boxedValue and return the same if exists else return the same value class.

Earlier

return if (resultType == expectedReturnType && !(isReturnNullable && isPrimitive)) {
    this.boxedValue
} else {
    this
}

Now

return if (resultType == expectedReturnType && !(isReturnNullable && isPrimitive)) {
    this.boxedValue
 } else {
    this.boxedValue ?: this
 }

@kpadhiamex kpadhiamex closed this Dec 19, 2024
@kpadhiamex kpadhiamex deleted the 1330-fix-for-relaxed-mocking-simple-value-when-property-is-nested-value-class branch December 19, 2024 12:48
@kpadhiamex kpadhiamex restored the 1330-fix-for-relaxed-mocking-simple-value-when-property-is-nested-value-class branch December 19, 2024 16:55
@kpadhiamex kpadhiamex reopened this Dec 19, 2024
@kpadhiamex kpadhiamex changed the title Issue (no. 1330) Fix for Relaxed Mocking Value When Property is Nested Value Class Fix(Issue no. 1330) for Relaxed Mocking Value When Property is Nested Value Class Dec 19, 2024
@kpadhiamex
Copy link
Contributor Author

@Raibaz Can I ask, kindly review this PR if you are available?

@Raibaz
Copy link
Collaborator

Raibaz commented Dec 20, 2024

Hi @kpadhiamex, thanks for looking into this!

PR looks good to me, but to merge it I need the builds to pass. Can you please rebase on top of master? I just pushed a fix to the build process.

@Raibaz Raibaz self-requested a review December 20, 2024 10:40
@kpadhiamex
Copy link
Contributor Author

Hi @Raibaz , Many thanks for your comments and approval. I just merged master branch, could you please have a look and approve the workflow which is waiting for approval?

1 workflow awaiting approval

@Raibaz Raibaz merged commit e7ec061 into mockk:master Dec 20, 2024
22 checks passed
@kpadhiamex kpadhiamex deleted the 1330-fix-for-relaxed-mocking-simple-value-when-property-is-nested-value-class branch December 20, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants