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

Additional Verify overload #1463

Merged
merged 1 commit into from
Jun 20, 2024
Merged

Conversation

bkijonka
Copy link
Contributor

@bkijonka bkijonka commented Mar 1, 2024

No description provided.

@CLAassistant
Copy link

CLAassistant commented Mar 1, 2024

CLA assistant check
All committers have signed the CLA.

@kzu
Copy link
Member

kzu commented Jun 14, 2024

What scenario does this cover that can't be already be enabled by an extension method in consumer code that does the same?

@bkijonka
Copy link
Contributor Author

What scenario does this cover that can't be already be enabled by an extension method in consumer code that does the same?

It covers the following scenario:

interface IRepo
{
    Task<object> GetSomething();
}


[TestMethod]
public async Task Test()
{
    // Arrange
    var mock = new Mock<IRepo>();

    // Assert
    mock.Verify(repository => repository.GetSomething(), Times.Once, "Fail message");
}

If you write the code as above, you will get the analyzer warning VSTHRD110 "Observe the awaitable result of this method call by awaiting it, assigning to a variable, or passing it to another method"

We currently use a work around by adding () to Times.Once or Times.Never, like below:
mock.Verify(repository => repository.GetSomething(), Times.Once(), "Fail message");

but this may be missed by developers and lead to analyzer warnings.

The overload is simply missing and should be added.

@kzu
Copy link
Member

kzu commented Jun 20, 2024

Time.Once is a method. It's not clear to me why someone would use it as a property.

But it's low hanging fruit so probably makes sense.

@kzu kzu force-pushed the additional_verify_overload branch from 3cacc6c to cc153db Compare June 20, 2024 05:31
@kzu kzu enabled auto-merge (rebase) June 20, 2024 05:32
@kzu kzu merged commit 6e407a0 into devlooped:main Jun 20, 2024
4 checks passed
@devlooped devlooped locked and limited conversation to collaborators Sep 4, 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

Successfully merging this pull request may close these issues.

None yet

3 participants