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

Interface inheritance introduced a possible breaking change in 4.2.1408.619 #371

Closed
tupacko opened this issue Jun 12, 2017 · 2 comments
Closed

Comments

@tupacko
Copy link

tupacko commented Jun 12, 2017

Hello,

our project tried to upgrade to the latest Moq and found an issue. Lots of our tests started failing with regards to property change notification. After some digging we isolated the issue and found the culprit. Of course, what we do not know why it was introduced or what else would be broken by it's removal.

Basically, after 4.2.1402.2112, starting with 4.2.1408.619, the following code changes it's behavior:

public class Program
  {
    static void Main(string[] args)
    {
      EventHolderConcrete testSubject = new Mock<EventHolderConcrete> { CallBase = true }.Object;
      var eventConsumer = new EventConsumer(testSubject);

      testSubject.Something = "something";

      Console.Out.WriteLine($"{eventConsumer.Count}, should be 1");
      Console.ReadKey(true);
    }
    
    public interface IEnhancedNotifyPropertyChanged : INotifyPropertyChanged
    {
    }

    public abstract class EventHolderBase : IEnhancedNotifyPropertyChanged
    {
      public event PropertyChangedEventHandler PropertyChanged;
      
      protected void OnPropertyChanged(string propertyName)
      {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
      }
    }

    public class EventHolderConcrete : EventHolderBase
    {
      public string Something
      {
        get
        {
          return this.something;
        }
        set
        {
          this.something = value;
          OnPropertyChanged(nameof(Something));
        }
      }

      private string something;
    }

    public class EventConsumer
    {
      public EventConsumer(INotifyPropertyChanged eventSource)
      {
        eventSource.PropertyChanged += EventSource_PropertyChanged;
      }
      
      public int Count { get; set; }
      
      private void EventSource_PropertyChanged(object sender, PropertyChangedEventArgs e)
      {
        Count++;
      }
    }
  }

The line in the Moq source which triggers this change in behavior is:
this.ImplementedInterfaces.AddRange(typeof(T).GetInterfaces().Where(i => (i.IsPublic || i.IsNestedPublic) && !i.IsImport));

It was introduced at line 115 in Mock.Generic.cs with the tag version 4.2.1408.619.

This issue might be related to: #275 Still, this is more generic as one do not need to have the same property on two interface ... a simple interface inheritance kills the tests.

Hope the bug report and the investigation will help fixing this.

Thank you,
Alpár

@stakx
Copy link
Contributor

stakx commented Jun 19, 2017

Hi @tupacko, thanks for the report and the analysis. I've tracked down a whole group of issues to the same line of code.

However, the code you provided doesn't appear to suffer from it. When I run it, using Moq 4.7.49 (the current version at this time), its output is:

1, should be 1

Unless that is supposed to be the error message :) I cannot actually reproduce a problem here, so I am closing this for now.

If I have misunderstood something here, or you would like to add different repro code, please just report back and we can reopen the issue.

@stakx stakx closed this as completed Jun 19, 2017
@tupacko
Copy link
Author

tupacko commented Jun 20, 2017

Hi,

you are right, it has been fixed in 4.5.22. Unfortunately we cannot update the project to 4.5 as of now (we have still to support .NET 4 for some time). Will keep you updated if we find any issues on the latest version after we upgrade!

Thank you for your answer!

@devlooped devlooped locked and limited conversation to collaborators Sep 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants