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

Added new test for creation of proxies of instances with properties that are assigned a value from the constructor. #15

Closed
wants to merge 1 commit into from

Conversation

darkcamper
Copy link

When I try to create a proxy from a class that assigns a value to a property from the constructor a NullReferenceException is thrown.

I know that calling virtual member functions from the constructor can cause problems, but the property is only virtual because it is necessary for proxy creation.

I have added a new test to ProxyCreationTest.cs that reproduces this issue.

…hat are assigned a value from the constructor.
@mfidemraizer
Copy link
Owner

@darkcamper Hey ;) I'll take a look at this tomorrow! Thanks :D

@darkcamper
Copy link
Author

I've created an issue #16 to further discuss the problem there 

@mfidemraizer
Copy link
Owner

mfidemraizer commented Aug 8, 2016

@darkcamper Hey ;) I managed to fix the whole bug, It was an easy one.

BTW, the test included in your pull request is flawed, because your assertion is wrong: it's asserting if the non-proxy should be a proxy.

I give you an enhanced test:

    [TestMethod]
    public void CanCreateProxyWhenPropertyInitializedFromConstructor()
    {
        D d1 = new D();
        D trackedD1 = TrackableObjectFactory.CreateFrom(d1);

        trackedD1.Text = "bye bye";

        IObjectChangeTracker changeTracker = trackedD1.GetChangeTracker();

        Assert.AreEqual(1, changeTracker.ChangedProperties.Count);
    }

Once you've copy-pasted the whole test, add the change with git add . and git commit it.

Now you'll squash this last commit into the previous one, to merge both into a single commit. To do so, perform this steps:

  1. In your fork execute this command in the root dir: git rebase -i HEAD~2
  2. In the text editor, press I to enter to the interactive mode and being able to edit.
  3. Locate the last commit (it should be the last of two commits). Change the word pick with squash.
  4. Press ESC
  5. Type wq and press ENTER. Wait until the operation finishes.
  6. Now you need to push the rewritten history to your remote: git push --force.

If you could push your updated branch to your remote on your fork, now the associated commit with this pull request will contain the right test code and I'll be able to merge it on v2.

Thank you and I'll be waiting for this to push the bugfix!

@mfidemraizer
Copy link
Owner

See close details on #16

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

Successfully merging this pull request may close these issues.

2 participants