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

Can ValueTask IReturnsResult<TMock> extensions be added? #437

Closed
AdamDotNet opened this issue Aug 31, 2017 · 5 comments
Closed

Can ValueTask IReturnsResult<TMock> extensions be added? #437

AdamDotNet opened this issue Aug 31, 2017 · 5 comments
Assignees
Milestone

Comments

@AdamDotNet
Copy link
Contributor

The ReturnsAsync() and ThrowsAsync() are great, but sadly don't work with the newer type, ValueTask<T>. I've cooked up a gist that shows it pretty simple to add (most of the changes were Task to ValueTask), though I didn't peruse through your unit tests, the cost could be a little higher to implement. Also, Moq would have to depend on the System.Threading.Tasks.Extensions NuGet. That seems like a small barrier, given it supports .NetStandard1.0.

The idea is to merge in those extensions with your existing ReturnsExtension class.

I can live with adding these extensions to my own project or private NuGet, but it seems like a value add for everyone who uses the excellent Moq. Thank you!

@stakx
Copy link
Contributor

stakx commented Oct 24, 2017

@AdamDotNet - sounds like a sensible API enhancement. A couple of (fairly straightforward and unproblematic) things to keep in mind:

  • The additional dependency on System.Threading.Tasks.Extensions is not a problem, if we can still target .NET 4.5 and .NET Standard 1.3.

  • The new extension methods for ValueTask<T> should not cause method resolution conflicts in existing code.

  • There should be a few tests (in ReturnsExtensionsFixture.cs) targeting the new extension methods.

Would you like to submit a PR?

(If so, please target the develop branch.)

We should also consider feature parity with Task<T> support. Ideally, Moq would have equal support for both ValueTask<T> and Task<T>. I'd say let's start with ReturnsExtensions, then go through other parts of Moq in a second stage.

@AdamDotNet
Copy link
Contributor Author

@stakx - I'd like to be able to help. However, I can't quite get in a compiling state before I get to work on it.

GenericTypeParameters.tt Initially couldn't run, with the message 'Enumerable' does not exist in the current context.

I tested adding

<#@ Assembly Name="System.Core.dll" #>
<#@ import namespace="System.Linq" #>

The template now runs, but the resulting GenericTypeParameters.cs file is empty. If this file is not critical for what I'll be working on, I guess I'll proceed, and be sure to commit only the relevant file changes. Do you have any additional advice for working in the Moq.sln solution? Thanks!

@stakx
Copy link
Contributor

stakx commented Oct 24, 2017

@AdamDotNet: Given that you are using VS 2017, you should not have any problems with Moq.sln. I continue to be amazed by all the tooling-related unreliability that's cropped up lately in the .NET ecosystem and I have no idea what went wrong in your case.

If you're on a Windows box, keep in mind that depending on how you downloaded Moq source code, Windows might block it (for security reasons) and you first need to "unblock" files in Explorer. Perhaps that's why your template doesn't run correctly?

You can work without a functioning GenericTypeParameters.tt, but you won't be able to test the changes you're likely to have to make in RetuensExtensions.tt.

@AdamDotNet
Copy link
Contributor Author

@stakx - In short, ReturnsExtensions.tt generates, but here was my procedure:

  1. Fork to my repository
  2. Create valuetask-extensions branch off of develop.
  3. In VS 2017 (15.4.1) on Windows 10 16299, use GitHub extension to clone my repository and switch to valuetask-extensions branch. Seems like it wouldn't be a file blocked issue since I didn't download as Zip file.

Since it seems like I can write my extensions, I'll go ahead and proceed.

@stakx stakx added this to the v4.8.0 milestone Oct 24, 2017
@stakx stakx closed this as completed Oct 24, 2017
@stakx
Copy link
Contributor

stakx commented Dec 8, 2017

@AdamDotNet , @JonHeaton - I just published a pre-release version 4.8.0-rc1 on NuGet. It has (among other things) support for ValueTask<> with Returns, as well as support for Task<> and ValueTask<> with DefaultValue.Mock. Feel free to test away.

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

2 participants