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

ReturnValue not set #1696

Closed
asgerhallas opened this issue Nov 13, 2019 · 13 comments
Closed

ReturnValue not set #1696

asgerhallas opened this issue Nov 13, 2019 · 13 comments
Labels
Milestone

Comments

@asgerhallas
Copy link
Contributor

asgerhallas commented Nov 13, 2019

It seems like the ReturnValue of ICompletedFakeObjectCall is not being set. This test fails:

[Fact]
public void RetrnValueIsSet()
{
    var fake = A.Fake<IInterface>();

    var returnValue = fake.Function();

    Fake.GetCalls(fake).Single().ReturnValue.ShouldBe(returnValue);
}

public interface IInterface
{
    object Function();
}

Am I misunderstanding something - or is this a bug?

Using FakeItEasy version 5.4.0 on netcore 3.0.

@asgerhallas
Copy link
Contributor Author

This started failing after upgrading from version 1.18.0 on .NET Framework 4.7.1.

(yes, I know, that update was a little overdue :) )

@blairconrad
Copy link
Member

Hi, @asgerhallas. Sorry for your pain.

I read the documentation for the member and it says

/// Gets the value set to be returned from the call.

Which could be interpreted to mean that the current behaviour is correct - no value was set to be returned, so we ended up creating a Dummy to return. So maybe it's doing an allowable thing?

I'm not really satisfied with that explanation, though. It seems cheap, especially since the behaviour changed at some point. Since this is a completed call, I think we should be giving the value that we returned, not some indication of what was configured to have been returned, if anything. @thomaslevesque?

@thomaslevesque
Copy link
Member

Looks like a bug to me. I think it should contain the value that was effectively returned.

@blairconrad blairconrad added this to the vNext milestone Nov 13, 2019
@blairconrad
Copy link
Member

blairconrad commented Nov 13, 2019

I propose we fix this in the next release (expected to be 6.0.0), but haven't much appetite for a 5.4.1 release, unless we have evidence it's a recently-introduced change. Which would really surprise me.

@asgerhallas
Copy link
Contributor Author

Thank you for the quick replies, and sorry for my late one.

I have just checked, this broke between 5.0.1 (works) and 5.1.0 (does not work). Is that of any help?

@thomaslevesque
Copy link
Member

I have just checked, this broke between 5.0.1 (works) and 5.1.0 (does not work). Is that of any help?

That's not what I observed... For me it's working with 5.1.0 and not in 5.1.1. The problem was introduced when fixing #1583.

@asgerhallas
Copy link
Contributor Author

Sorry, you are very correct, I must have messed up my nuget downgrade 😳

@blairconrad blairconrad mentioned this issue Nov 14, 2019
12 tasks
@blairconrad
Copy link
Member

Fixed in #1697. GitHub didn't close the issue automatically.

@thomaslevesque
Copy link
Member

GitHub didn't close the issue automatically.

Strange... Maybe it's because it wasn't merged to master?

@blairconrad
Copy link
Member

Oh, good point.

@afakebot
Copy link

This change has been released as part of FakeItEasy 5.4.1.

@asgerhallas
Copy link
Contributor Author

Awesome! Thank you! :)

@blairconrad
Copy link
Member

Thanks for reporting it, @asgerhallas. We appreciate hearing of problems, especially with easy to follow reproduction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants