-
-
Notifications
You must be signed in to change notification settings - Fork 802
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
Fixed issue #314 #704
Fixed issue #314 #704
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
@echo off | ||
set CONFIG=Debug | ||
set VisualStudioVersion=15.0 | ||
|
||
for /f "usebackq tokens=*" %%i in (`vswhere -latest -requires Microsoft.Component.MSBuild -property installationPath`) do ( | ||
set INSTALLDIR=%%i | ||
) | ||
|
||
"%INSTALLDIR%\MSBuild\%VisualStudioVersion%\bin\MSBuild.exe" ".\moq.sln" /t:restore /t:build | ||
|
||
if not %ERRORLEVEL%==0 goto fail | ||
|
||
echo %time% | ||
echo. | ||
color | ||
|
||
pause | ||
exit /b 0 | ||
|
||
:end | ||
|
||
:fail | ||
color 07 | ||
|
||
echo. | ||
echo. | ||
echo Failed | ||
echo. | ||
pause | ||
exit /b 1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,6 +57,11 @@ public static bool IsPropertyIndexerSetter(this MethodInfo method) | |
return method.Name.StartsWith("set_Item", StringComparison.Ordinal); | ||
} | ||
|
||
public static bool IsPropertyIndexer(this MethodInfo method) | ||
{ | ||
return method.IsPropertyIndexerGetter() || method.IsPropertyIndexerSetter(); | ||
} | ||
|
||
public static bool IsPropertySetter(this MethodInfo method) | ||
{ | ||
return method.Name.StartsWith("set_", StringComparison.Ordinal); | ||
|
@@ -91,7 +96,7 @@ public static void ThrowIfNotMockeable(this MemberExpression memberAccess) | |
|
||
public static bool IsMockeable(this Type typeToMock) | ||
{ | ||
// A value type does not match any of these three | ||
// A value type does not match any of these three | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert this white space-only change. |
||
// condition and therefore returns false. | ||
return typeToMock.GetTypeInfo().IsInterface || typeToMock.GetTypeInfo().IsAbstract || typeToMock.IsDelegate() || (typeToMock.GetTypeInfo().IsClass && !typeToMock.GetTypeInfo().IsSealed); | ||
} | ||
|
@@ -293,4 +298,4 @@ private static void AddPropertiesInDepthFirstOrderTo(this Type type, List<Proper | |
} | ||
} | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert this line separator-only change. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,10 +100,10 @@ private MethodInfo GetTargetMethod(Type objectType, Type returnType) | |
{ | ||
//.Setup(mock => mock.Solution) | ||
return typeof(Mock<>) | ||
.MakeGenericType(objectType) | ||
.GetMethods("Setup") | ||
.First(m => m.IsGenericMethod) | ||
.MakeGenericMethod(returnType); | ||
.MakeGenericType(objectType) | ||
.GetMethods("Setup") | ||
.First(m => m.IsGenericMethod) | ||
.MakeGenericMethod(returnType); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert this white space-only change. |
||
} | ||
else | ||
{ | ||
|
@@ -115,11 +115,11 @@ private MethodInfo GetTargetMethod(Type objectType, Type returnType) | |
|
||
// Args like: string IFoo (mock => mock.Value) | ||
private Expression TranslateFluent(Type objectType, | ||
Type returnType, | ||
MethodInfo targetMethod, | ||
Expression instance, | ||
ParameterExpression lambdaParam, | ||
Expression lambdaBody) | ||
Type returnType, | ||
MethodInfo targetMethod, | ||
Expression instance, | ||
ParameterExpression lambdaParam, | ||
Expression lambdaBody) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert this white space-only change. |
||
{ | ||
var funcType = typeof(Func<,>).MakeGenericType(objectType, returnType); | ||
|
||
|
@@ -166,7 +166,9 @@ private static Mock<TResult> FluentMock<T, TResult>(Mock<T> mock, Expression<Fun | |
|
||
Mock fluentMock; | ||
MockWithWrappedMockObject innerMock; | ||
if (mock.InnerMocks.TryGetValue(info, out innerMock)) | ||
|
||
//When property is indexer, the method name is same, in this case i have return other instance of mock. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: |
||
if (!info.IsPropertyIndexer() && mock.InnerMocks.TryGetValue(info, out innerMock)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change fixes the tests you added, but unfortunately it breaks a related scenario: public interface IA
{
IB this[int index] { get; }
}
public interface IB
{
string X { get; }
string Y { get; }
}
[Fact]
public void Fixed_by_this_change()
{
var mock = Mock.Of<IA>(m => m[0].X == "0X" && m[1].X == "1X");
Assert.Equal("0X", mock[0].X);
Assert.Equal("1X", mock[1].X);
}
[Fact]
public void Broken_by_this_change()
{
var mock = Mock.Of<IA>(m => m[0].X == "0X" && m[0].Y == "0Y");
Assert.Equal("0X", mock[0].X);
Assert.Equal("0Y", mock[0].Y);
} I fear that the problem here is the whole |
||
{ | ||
fluentMock = innerMock.Mock; | ||
} | ||
|
@@ -191,4 +193,4 @@ private static Mock<TResult> FluentMock<T, TResult>(Mock<T> mock, Expression<Fun | |
return (Mock<TResult>)fluentMock; | ||
} | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert this line separator-only change. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
using Xunit; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now, we have one single file for all regression tests coming from GitHub issues; Could you please move these tests there? Thanks! (Btw., the following hint is not relevant if you move your tests to +// Copyright (c) 2007, Clarius Consulting, Manas Technology Solutions, InSTEDD.
+// All rights reserved. Licensed under the BSD 3-Clause License; see License.txt.
+ |
||
|
||
namespace Moq.Tests.Regressions | ||
{ | ||
public class FluentMockVisitorIssues | ||
{ | ||
[Fact] | ||
public void WhenQueryingWithTwoIndexes_ThenSetsThemDirectly1() | ||
{ | ||
var foo = Mock.Of<IFoo>(x => x[0].Value == "hello" && x[1].Value == "goodbye"); | ||
|
||
Assert.Equal("hello", foo[0].Value); // Fails here as foo[0] is the same object as foo[1] and foo[1].Value == "goodbye", Fixed! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "fails here" comment will no longer make sense once you've fixed the issue. Please remove it. (With all tests, the basic assumption is that they will pass, so there should be no comment saying "this fails". ;-) |
||
Assert.Equal("goodbye", foo[1].Value); | ||
} | ||
|
||
[Fact] | ||
public void WhenQueryingWithTwoIndexes_ThenSetsThemDirectly2() | ||
{ | ||
var foo = Mock.Of<IFoo>(x => x[0] == Mock.Of<IBar>(b => b.Value == "hello") && x[1] == Mock.Of<IBar>(b => b.Value == "goodbye")); | ||
|
||
Assert.Equal("hello", foo[0].Value); // These pass no problem | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar as in the above comment, there is no need to say "these pass no problem" — that is what we assume for all tests, anyway. (I know you copy & pasted these tests from the issue, and that is perfectly fine. Just pointing out that they often need to be changed a tiny bit so that they make sense in their new context.) |
||
Assert.Equal("goodbye", foo[1].Value); | ||
} | ||
|
||
public interface IFoo | ||
{ | ||
IBar this[int index] { get; } | ||
} | ||
|
||
public interface IBar | ||
{ | ||
string Value { get; } | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this batch file and the
vswhere.exe
binary, for two reasons: (1) I recently took pains to get rid of unnecessary build scripts, I wouldn't want to go back to a batch file. (2) Fixing the build is unrelated to the issue you're trying to solve.If you have problems building Moq, then please raise a new issue so we can look at it.