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

4.7.137 regression #469

Closed
cbruun opened this issue Oct 4, 2017 · 9 comments
Closed

4.7.137 regression #469

cbruun opened this issue Oct 4, 2017 · 9 comments

Comments

@cbruun
Copy link

cbruun commented Oct 4, 2017

After upgrading to Moq 4.7.137 the following scenario breaks:

public void Main()
{
	new Mock<IBreak>().Object;
}

public interface IBreak
{
	void DoStuff<T>(Implementation1<T> a);
	void DoStuff<T>(Implementation2<T> a);
}

public abstract class BaseType<T>
{
}

public class Implementation1<T> : BaseType<T>
{
}

public class Implementation2<T> : BaseType<T>
{
}
@stakx
Copy link
Contributor

stakx commented Oct 4, 2017

Thanks for reporting. This looks like a valid usage scenario, I can't see a reason why this shouldn't work.

It appears to be caused by some change inside Castle Core 4.2.0. I'll do some investigation and see what I can find.

@stakx
Copy link
Contributor

stakx commented Oct 4, 2017

@cbruun, most of the work to get this fixed will have to happen over at castleproject/Core now.

In the meantime, if you could spare some time & would like to contribute to Moq, please feel free to submit a PR (against the develop branch) that adds your above repro code as a skipped unit test there, in UnitTests/Regressions/IssueReportsFixture.cs. For example:

#region 469

public class Issue469
{
    [Fact(Skip = "Temporarily disabled due to a regression in Castle Core, see https://github.com/castleproject/Core/issues/309.")]}

#endregion

P.S.: BaseType<T> isn't actually needed to reproduce the problem.

@ADThomsen
Copy link

@stakx, @cbruun and I worked on the issue together when he reported the regression, so I have taken the liberty to write the test and make the pull request.

I just now saw that you mention BaseType<T> isn't actually needed. Feel free to remove it from the PR.

@stakx
Copy link
Contributor

stakx commented Oct 5, 2017

@ADThomsen, thanks for letting me know. Will merge your PR in a short while.

@ivancitin
Copy link

This version along with Castle.Core 4.2.0 definitely breaks us. Will be stock with the previous version, i'll give it a try on the next release.

I was not able to pinpoint the issue, other than noticing test get hanged indefinitely.

@stakx
Copy link
Contributor

stakx commented Oct 6, 2017

@ivancitin: Can you please post repro code of such a test that hangs as a separate issue? Could be a different problem than the one reported here.

@stakx
Copy link
Contributor

stakx commented Oct 10, 2017

I'm preparing a "hotfix" version of Moq (4.7.x) which should be available in the next 24 hours or so.

@stakx stakx closed this as completed Oct 10, 2017
@stakx
Copy link
Contributor

stakx commented Oct 10, 2017

@cbruun and @ADThomsen: Moq 4.7.142 is now available. Once you update (which should automatically update its dependency Castle Core to 4.2.1, which is the important bit), you should be good.

@ADThomsen
Copy link

@stakx Thanks for the update. We have upgraded Moq and can confirm that our tests again work as expected.

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

No branches or pull requests

4 participants