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

Thread safety #1

Closed
wants to merge 6 commits into from
Closed

Thread safety #1

wants to merge 6 commits into from

Conversation

FelicePollano
Copy link
Contributor

Hi Damian,
I added thread safety support. There is also a new unit test show it works ( other test still passes ;) )
Please let me know if you like it.
Thanks!

@damianh
Copy link

damianh commented Mar 13, 2012

First pull request for the Moq4 line... nice!

Before I pull, I'm going to verify with Daniel what our strategy is with this. Clearly he is thinking about a clean start with Moq5. I'm more interested at this point at getting a Moq4 maintenance release out. There are a couple of things I'd like to do before modifying the existing code base such as getting it running under CI etc.

Leave it with me, will get back to you soon (ish)

@FelicePollano
Copy link
Contributor Author

Thank you Damian. This is perfectly OK. I will push in the maintenance repo if you/Daniel agree, because I think this is useful ( even in MoQ 5 it would be nice ) Current push is thread safe at the infrastructure level, and this worked for me. I'm just doing an adjustement, that I think is necessary ( and maybe you would consider even in Moq 5) to setup the moq to be SingleThreaded ( in a per method base ) so you can ensure the SUT is calling such a method/property not in multithread ( this is usefule when the actual object isn't thread safe and you want to ensure it is called single threaded )

@kzu
Copy link
Contributor

kzu commented Mar 21, 2012

I guess I missed the notifications here because this was on another repo before :)

I'd make the Moq infrastructure itself thread safe.

@kzu
Copy link
Contributor

kzu commented Mar 21, 2012

Also, the pull requests should go against dev branch (just created it).

@FelicePollano
Copy link
Contributor Author

Thank you Daniel,
 So you will ignore my push and simply modify the code, correct ?
Anyway well to know, before I push in the only branch there is in the project.I did some other mod in my fork too, have a look if you feel there are useful: http://www.felicepollano.com/2012/03/21/Moq4ThreadSafetyAndMore.aspxMy anglish is bad, I'm sorry in advance for that, but let me know,
Regards,

 

Felice Pollano
Senior IT Architect & Developer

http://www.felicepollano.com

Da: "Daniel Cazzulino" reply@reply.github.com
A: "FelicePollano" felice@felicepollano.com
Cc:
Data: Wed, 21 Mar 2012 10:58:14 -0700
Oggetto: Re: [moq4] Thread safety (#1)

Also, the pull requests should go against dev branch (just created it).


Reply to this email directly or view it on GitHub:
#1 (comment)

@kzu
Copy link
Contributor

kzu commented Mar 21, 2012

I think we should separate the previous pull request (just fixing the multi-threading) from the SingleThreaded extension method. (I'm not so sure about the usefulness of that in the "core" moq).

Also, any chance you can keep the tab/spaces on the file intact? it's hard to diff in github site when those are changed. The entire file just appears deleted/added :(

@damianh
Copy link

damianh commented Mar 21, 2012

Gents (slightly OT here), when you run the build script / run the unit tests in VS what is your test count?

@FelicePollano
Copy link
Contributor Author

@danielkzu I will fix this with a new pull, I'm sorry, I wiull check in a private repo to see the proper cr-lf config. For sure the two pull request are separated, the SingleThreaded version maybe is just useful to me, in this case forget about it I will keep it in my private fork :)

@FelicePollano
Copy link
Contributor Author

@damianh I do use the xUnit external GUI ( I'have not resharper ) anyway My test count is 606 total ( 4 are mine for the mt stuff ) 9 test appear to be skipped. Additionally, there is another test that requires office interop that I have #ifdeffed out on my fork. Hope this help :)

@FelicePollano
Copy link
Contributor Author

Closing! Fix the CR-LF stuff and work on the proper trunk

@FelicePollano
Copy link
Contributor Author

Please I'm a bit confused on the crlf stuff: http://help.github.com/line-endings/ My current git settings are AutoCrlLf on and SaveCrLf ON, please tell me the correct ones :)

@kzu
Copy link
Contributor

kzu commented Mar 22, 2012

Should be all false,
On Mar 22, 2012 5:04 AM, "FelicePollano" <
reply@reply.github.com>
wrote:

Please I'm a bit confused on the crlf stuff:
http://help.github.com/line-endings/ My current git settings are
AutoCrlLf on and SaveCrLf ON, please tell me the correct ones :)


Reply to this email directly or view it on GitHub:
#1 (comment)

@FelicePollano
Copy link
Contributor Author

Thank you, I retryed the pull with that settings and should be ok now :) I'm interested in opinion about the SelfThread() expectation. You can point for opinions my post if you want.

kzu pushed a commit to kzu/moq4 that referenced this pull request May 24, 2016
Merge master into netcore for PR
kzu pushed a commit that referenced this pull request Nov 10, 2016
T4-generated overloads for ReturnsAsync(Func<T1, T2, ... TResult> value)
stakx pushed a commit that referenced this pull request Oct 31, 2022
david-kalbermatten added a commit to david-kalbermatten/moq4 that referenced this pull request Jun 9, 2023
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

Successfully merging this pull request may close these issues.

None yet

3 participants