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

Problems with Task in 4.2 version #78

Closed
mmajcica opened this issue Dec 20, 2013 · 10 comments
Closed

Problems with Task in 4.2 version #78

mmajcica opened this issue Dec 20, 2013 · 10 comments
Milestone

Comments

@mmajcica
Copy link

since I updated to the latest version of Moq (4.2) I'm having some unit test failing. I'm getting the System.InvalidCastException: Unable to cast object of type 'FilterCriteria' to type 'SortCriteria'. As my code uses Task's to get these values I suspect the new version of Moq works differently than previous one.

This is the code in question:

In tested class:

Task<FilterCriteria> getFilterCriteriaTask = Task<FilterCriteria>.Factory.StartNew(() => searchService.GetFilterCriteriaFromCache(id));
Task<SortCriteria> getSortCriteriaTask = Task<SortCriteria>.Factory.StartNew(() => searchService.GetSortCriteriaFromCache(id));

FilterCriteria filterCriteria = getFilterCriteriaTask.Result;
SortCriteria sortCriteria = getSortCriteriaTask.Result;

In unit test:

searchServiceMock
   .Setup(it => it.GetFilterCriteriaFromCache(expectedCacheKey, null))
   .Returns(expectedFilterCriteria);

searchServiceMock
   .Setup(it => it.GetSortCriteriaFromCache(expectedCacheKey))
   .Returns(expectedSortCriteria);

Any idea on what it can be due to? I really do not know where to start.

Thanks

@kzu
Copy link
Member

kzu commented Dec 20, 2013

@alextercete or @Blewzman: do you have an idea of what the issue might be?

@alextercete
Copy link

@mmajcica, are GetSortCriteriaFromCache and GetFilterCriteriaFromCache async? I mean, do they return Task or Task<>?

@mmajcica
Copy link
Author

@alextercete This is the signature of both of the methods.

SortCriteria GetSortCriteriaFromCache(string cacheKey);
FilterCriteria GetFilterCriteriaFromCache(string cacheKey, FlowType? flowType = null);

Seems that issue is due to the fact that moq in this version treats the threads in a different way.

Test didin't changed and if I revert the mock to 4.0 it is green.

@mmajcica
Copy link
Author

This evening I will try to replicate with a small example and come back with more details.

@alextercete
Copy link

@danielkzu, I think the issue here is not related to the recent async improvements in Moq. It might be related to #68 though.

@kzu
Copy link
Member

kzu commented Dec 20, 2013

@MatKubicki you think this might be related?

@MatKubicki
Copy link
Contributor

I've tried what I assume is a set of tests similiar to the code mmajcica has and it fails in the way reported. And when I add the lock in the Interceptor that was removed for #68 it passes.

So this seems to be a problem with handling the setups when they are run in multiple threads. I will see if I will have a look at sorting this out, although this is my last day working.

For reference this test seems to replicate the issue:

    public interface IInterfaceOne
    {
        ResultOne GetResultOne(string cacheKey);
        ResultTwo GetResultTwo(string cacheKey, int? otherKey = null);
    }

    public class ResultOne
    {

    }
    public class ResultTwo
    {

    }

    public class TestOne
    {
        public void TestMethod(IInterfaceOne intOne, string key)
        {
            Task<ResultTwo> getResultTwoTask = Task<ResultTwo>.Factory.StartNew(() => intOne.GetResultTwo(key));
            Task<ResultOne> getResultOneTask = Task<ResultOne>.Factory.StartNew(() => intOne.GetResultOne(key));

            ResultTwo filterCriteria = getResultTwoTask.Result;
            ResultOne sortCriteria = getResultOneTask.Result;
        }

    }

    public class TestCase
    {
        [Fact()]
        public void DoTest()
        {
            Mock<IInterfaceOne> m = new Mock<IInterfaceOne>();

            ResultOne expectedResOne = new ResultOne();
            ResultTwo expectedResTwo = new ResultTwo();

            m.Setup(it => it.GetResultTwo("Test", null)).Returns(expectedResTwo);
            m.Setup(it => it.GetResultOne("Test")).Returns(expectedResOne);


            TestOne t = new TestOne();
            t.TestMethod(m.Object, "Test");

            m.VerifyAll();
        }
    }

@kzu
Copy link
Member

kzu commented Dec 20, 2013

Thanks for taking the time to look at this issue @MatKubicki !

@MatKubicki MatKubicki mentioned this issue Dec 22, 2013
@MatKubicki
Copy link
Contributor

On the above PR, I had a few go's at fixing this problem. The root of it is that the CurrentCall property on the InterceptorStrategyContext was being set on multiple threads and then used later on, what I needed to add was a CurrentCall property for each thread working through Interceptor.Intercept. In the PR I've done this by splitting the InterceptorStrategyContext into a class used by all intercepts (InterceptorContext) and a class used by each thread in Intercept (CurrentInterceptContext).

The only thing on CurrentInterceptContext is the CurrentCall, I did try removing that entirely from the context as it can easily be pulled from the ordered calls list, I made the code in ExtractProxyCall to do this shared and this worked for all but one case.

This case was the MockSequence tests where getting the CurrentCall more than once resulted in incorrect behaviour and MockSequenceFixture.NoCyclicSequenceFail and CyclicSequenceSuccesss failing. The problem was checking the status of a Condition changes the sequence number, so checking more than once changes behaviour, there is a note in the original code indicating this slightly hacky nature! See MockSequence.For for this comment, I didn't have time to start tearing apart this code as well as the interceptor, so have gone with the easier fix described. If someone who knows their way around wants to take a shot over Christmas then go for it!

Hope this rambling was of use,
TL;DR; Problem's fixed but in the easiest way, some old code could be fixed to make the fix neater!

@mmajcica
Copy link
Author

mmajcica commented Jan 8, 2014

@danielkzu When will this be available via NuGet?

Thanks

@mmajcica mmajcica closed this as completed Jan 8, 2014
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

No branches or pull requests

4 participants