devlooped / moq

The most popular and friendly mocking framework for .NET
Other
5.94k stars 801 forks source link

Add support for `success` condition parameter in `When`. #1237

Closed ghost closed 2 years ago

ghost commented 2 years ago

The current MockSequence implementation looks like this:

internal ISetupConditionResult<TMock> For<TMock>(Mock<TMock> mock)
    where TMock : class
{
    var expectationPosition = sequenceLength++;

    return new WhenPhrase<TMock>(mock, new Condition(
        condition: () => expectationPosition == sequenceStep,
        success: NextStep));
}

Do do a custom similar thing we'd use the When method:

public ISetupConditionResult<T> When(Func<bool> condition)
{
    return new WhenPhrase<T>(this, new Condition(condition));
}

But it doesn't expose the success action. What I'm suggesting is to evolve this signature to:

public ISetupConditionResult<T> When(Func<bool> condition, Action success = null)
{
    return new WhenPhrase<T>(this, new Condition(condition, success));
}

I'd like to use this to setup a custom condition that depends on that success.

Thanks

stakx commented 2 years ago

That success thingy is an internal implementation detail, and ideally it wouldn't exist at all, because it is somewhat problematic from a thread-safety point of view. Therefore, exposing it does not seem a good idea.

Why don't you simply put your logic in a .Callback(...)?

ghost commented 2 years ago

@stakx so why is Moq using that internal implementation instead on .Callback(...)? I thought that maybe it is because .Callback is usable only once per setup and you don't want to fill that space during InSequence.

I'm try to make my custom sequencer InMyCustomSequencer so that the developer can do something like:

mock.InMyCustomSequencer(....)
    .Setup(x => x.Method())
    .Returns(10)
    .Callback(() => developerChoosenCallback())

Basically I'm augmenting Moq features available to a developer to cover a particular use case.

ghost commented 2 years ago

Actually now I see, Callback is not usable from the "sequencer" because the Setup has not been executed yet. And I don't know which method will the developer choose to setup. And that's the reason because I'm asking to expose that success parameter.

stakx commented 2 years ago

Not sure what your "sequencers" are exactly, but if they're in any way related or similar to MockSequence, then it's no wonder you ended up with setup.When(...) – right now, it's the only way to implement sequences.

That being said, Moq's sequences are very limited, and conditional setups that they're based on have a whole bunch of warts.

Instead of adding another wart to conditional setups (exposing the internal success callback) and then having to maintain that new public API indefinitely, we should figure out a better way how to support setting up & verifying a sequence of calls, and focus on that. See e.g. #75 about that.

ghost commented 2 years ago

My goal is to test a service call that internally uses an unit of work multiple times to call repositories.

void SomeServiceMethod()
{
    uow.BeginTran();
    repo1.DoThings();
    repo2.DoOtherThings();
    uow.Commit();

    // do other things

    uow.BeginTran();
    repo3.DoSomething();
    repo4.DoEverything();
    uow.Commit();
}

Of course I we need to mock UnitOfWork and all of the repositories to make a good unit test.

I want to verify that repositories are being called in the same transaction in the correct order.

mockRepo1.InTransaction(mockUnitOfWork).Setup(x => x.DoThings(), transactionIndex: 0).Verifiable();
mockRepo2.InTransaction(mockUnitOfWork).Setup(x => x.DoOtherThings(), transactionIndex: 0).Verifiable();

mockRepo3.InTransaction(mockUnitOfWork).Setup(x => x.DoSomething(), transactionIndex: 1).Verifiable();
mockRepo4.InTransaction(mockUnitOfWork).Setup(x => x.DoEverything(), transactionIndex: 1).Verifiable();

At the moment I'm using reflection to make use of that hidden parameter: I know this is a horrible practice.

/// <summary>
/// Internal <see cref="Moq.Language.Flow.WhenPhrase{T}"/> class used by sequence
/// </summary>
private readonly static Type _whenPhraseGenericType = typeof(Mock).Assembly.GetTypes().First(t => t.FullName == "Moq.Language.Flow.WhenPhrase`1");

/// <summary>
/// Internal <see cref="Condition"/> class used by sequence
/// </summary>
private readonly static ConstructorInfo _conditionCtor = typeof(Mock).Assembly.GetTypes().First(t => t.FullName == "Moq.Condition").GetConstructor(new[] { typeof(Func<bool>), typeof(Action) })!;

internal ISetupConditionResult<TMock> For<TMock>(Mock<TMock> mock, int transactionIndex, bool verifySequence) where TMock : class
{
    while (this._sequenceLengths.Count <= transactionIndex)
    {
        this._sequenceLengths.Add(0);
    }

    int expectationPosition = this._sequenceLengths[transactionIndex]++;

    //var condition = new Condition(() => this._transactionIndex == transactionIndex && (expectationPosition == this._sequenceStep || !verifySequence), NextStep);
    object condition = _conditionCtor.Invoke(new object[] { () => this._transactionIndex - 1 == transactionIndex && (expectationPosition == this._sequenceStep || !verifySequence), NextStep });
    //return new WhenPhrase<TMock>(mock, condition);
    return (ISetupConditionResult<TMock>)_whenPhraseGenericType.MakeGenericType(typeof(TMock)).GetConstructor(new[] { mock.GetType(), condition.GetType() })!.Invoke(new object[] { mock, condition });
}

I get what you're saying though. And I agree with you, there's should be a better way of customizing sequence logics.

I'm going to close this issue, and if I come up with an idea, I'll report it in the issue that you linked.