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 #328 #9

Merged
merged 1 commit into from Jun 18, 2012
Merged

Fix Issue #328 #9

merged 1 commit into from Jun 18, 2012

Conversation

yorah
Copy link
Contributor

@yorah yorah commented Jun 15, 2012

Make Mock.Of work on properties with non-public setters

@yorah
Copy link
Contributor Author

yorah commented Jun 15, 2012

Actually, the issue I encountered was not related to internal members, but to property protected setters, like:

public string MyProperty { get; protected set; }

The real fix to this issue is accomplished by passing true to GetSetMethod, so that it also looks for non-public methods (protected being non-public, as you can guess 😄).

However, as you say, the line I changed in CanOverride (adding !method.IsAssembly) will surely affect internal members and prevent them from working even with InternalsVisibleTo. I will look into it tomorrow, and rollback this particular changes if it indeed introduces a regression!

@yorah
Copy link
Contributor Author

yorah commented Jun 16, 2012

@danielkzu I just updated the PR so that it should keep working with internal members. Nice catch!

@kzu
Copy link
Contributor

kzu commented Jun 18, 2012

Looks good! Thanks!

kzu added a commit that referenced this pull request Jun 18, 2012
Fix Issue #328: Make Mock.Of work on properties with non-public setters
@kzu kzu merged commit cff47ed into devlooped:master Jun 18, 2012
@haacked
Copy link

haacked commented Jun 19, 2012

Was just about to report this. Has this been pushed to NuGet yet?

@kzu
Copy link
Contributor

kzu commented Jun 19, 2012

Nop, pushed to github yes. I'll try to push to nuget by the end of the week

/kzu from mobile
On Jun 19, 2012 7:14 PM, "Phil Haack" <
reply@reply.github.com>
wrote:

Was just about to report this. Has this been pushed to NuGet yet?


Reply to this email directly or view it on GitHub:
#9 (comment)

@haacked
Copy link

haacked commented Jun 19, 2012

Ok, there's still a bug in here. The following test fails. I think your tests pass because the class you're testing is in the same class as the test method. in this case, I made the class I'm testing in a separate class and this fails.

    public class Foo
    {
        protected Foo()
        {
        }

        public virtual string Value { get; private set;}
    }

    public class FooFixture
    {
        [Fact]
        public void Test()
        {
            var remote = Mock.Of<Foo>(rt => rt.Value == "foo");
            Assert.Equal("foo", remote.Value);

        }
    }

@haacked
Copy link

haacked commented Jun 19, 2012

Note that it fails if I remove the protected ctor in Foo too.

@haacked
Copy link

haacked commented Jun 19, 2012

This is weird. Running this in the the debugger shows that the method set_Value(System.String) is not virtual.

@kzu
Copy link
Contributor

kzu commented Jun 19, 2012

It's private, so it makes sense...

/kzu from mobile
On Jun 19, 2012 7:38 PM, "Phil Haack" <
reply@reply.github.com>
wrote:

This is weird. Running this in the the debugger shows that the method
set_Value(System.String) is not virtual.


Reply to this email directly or view it on GitHub:
#9 (comment)

@haacked
Copy link

haacked commented Jun 19, 2012

Ah right. But the getter should still be overriden. I opened issue #14 with the problem.

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.

None yet

3 participants